Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH v3 1/2] video: ARM CLCD: Add DT support
From: Russell King - ARM Linux @ 2013-09-23 16:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52406C97.3010800@wwwdotorg.org>

On Mon, Sep 23, 2013 at 10:30:15AM -0600, Stephen Warren wrote:
> On 09/23/2013 10:06 AM, Russell King - ARM Linux wrote:
> > On Mon, Sep 23, 2013 at 10:03:18AM -0600, Stephen Warren wrote:
> >> It sounds like you could just put LCDControl & 0x2e in the DT rather
> >> than using values such as 0x100..0x107, which don't appear to match the
> >> register format you mentioned above.
> > 
> > No.  Platforms which route the outputs to something like VGA or HDMI can
> > change the framebuffer format.  Your suggestions is far too restrictive.
> 
> Surely the DT should describe the HW setup only. Usually, a particular
> HW setup can support multiple different framebuffer formats. Hence, the
> DT wouldn't/shouldn't imply anything about the framebuffer format, but
> simply which wires are connected to the LCD.

Quite, and putting the contents of the LCDControl register - even just
bits 5 and 3-1 results in you having to modify the DT and reboot the
kernel just to change the framebuffer format.  That's why I'm objecting
to your comment.

When I rewrote the way the CLCD driver handles the various panels, I did
it with full information on how the hardware was being used at that time.
That is precisely why I came up with the capability system, where we
describe which formats the hardware can support up to the interface,
separately from the formats which the attached device - be it a LCD
panel, VGA socket or HDMI socket - can support.  The resulting set of
formats which can be used are a union of these.

Suggesting that we can do this by putting register values into DT is
completely wrong - if that were possible, I wouldn't have come up with
this capability system to sort this mess out in the first place - I
could've just hard-coded the register values and said to everyone
"tough, on these platforms you only get RGB444 support and that's it."

^ permalink raw reply

* Re: [PATCH v3 1/2] video: ARM CLCD: Add DT support
From: Stephen Warren @ 2013-09-23 16:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130923160640.GT12758@n2100.arm.linux.org.uk>

On 09/23/2013 10:06 AM, Russell King - ARM Linux wrote:
> On Mon, Sep 23, 2013 at 10:03:18AM -0600, Stephen Warren wrote:
>> It sounds like you could just put LCDControl & 0x2e in the DT rather
>> than using values such as 0x100..0x107, which don't appear to match the
>> register format you mentioned above.
> 
> No.  Platforms which route the outputs to something like VGA or HDMI can
> change the framebuffer format.  Your suggestions is far too restrictive.

Surely the DT should describe the HW setup only. Usually, a particular
HW setup can support multiple different framebuffer formats. Hence, the
DT wouldn't/shouldn't imply anything about the framebuffer format, but
simply which wires are connected to the LCD.

^ permalink raw reply

* Re: [PATCH] video: mmp: drop needless devm cleanup
From: Russell King - ARM Linux @ 2013-09-23 16:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1379952790-13202-1-git-send-email-u.kleine-koenig@pengutronix.de>

On Mon, Sep 23, 2013 at 06:13:10PM +0200, Uwe Kleine-König wrote:
> The nice thing about devm_* is that the driver doesn't need to free the
> resources but the driver core takes care about that. This also
> simplifies the error path quite a bit and removes the wrong check for a
> clock pointer being NULL.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/video/mmp/hw/mmp_ctrl.c | 17 ++---------------
>  1 file changed, 2 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/video/mmp/hw/mmp_ctrl.c b/drivers/video/mmp/hw/mmp_ctrl.c
> index 75dca19..6ac7552 100644
> --- a/drivers/video/mmp/hw/mmp_ctrl.c
> +++ b/drivers/video/mmp/hw/mmp_ctrl.c
> @@ -514,7 +514,7 @@ static int mmphw_probe(struct platform_device *pdev)
>  	if (IS_ERR(ctrl->clk)) {
>  		dev_err(ctrl->dev, "unable to get clk %s\n", mi->clk_name);
>  		ret = -ENOENT;
> -		goto failed_get_clk;
> +		goto failed;
>  	}
>  	clk_prepare_enable(ctrl->clk);
>  
> @@ -551,21 +551,8 @@ failed_path_init:
>  		path_deinit(path_plat);
>  	}
>  
> -	if (ctrl->clk) {
> -		devm_clk_put(ctrl->dev, ctrl->clk);
> -		clk_disable_unprepare(ctrl->clk);

And this patch also fixes the above: disabling/unpreparing _after_ putting
the thing - which was quite silly... :)

^ permalink raw reply

* [PATCH] video: mmp: drop needless devm cleanup
From: Uwe Kleine-König @ 2013-09-23 16:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1379951159-8294-1-git-send-email-u.kleine-koenig@pengutronix.de>

The nice thing about devm_* is that the driver doesn't need to free the
resources but the driver core takes care about that. This also
simplifies the error path quite a bit and removes the wrong check for a
clock pointer being NULL.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/mmp/hw/mmp_ctrl.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/video/mmp/hw/mmp_ctrl.c b/drivers/video/mmp/hw/mmp_ctrl.c
index 75dca19..6ac7552 100644
--- a/drivers/video/mmp/hw/mmp_ctrl.c
+++ b/drivers/video/mmp/hw/mmp_ctrl.c
@@ -514,7 +514,7 @@ static int mmphw_probe(struct platform_device *pdev)
 	if (IS_ERR(ctrl->clk)) {
 		dev_err(ctrl->dev, "unable to get clk %s\n", mi->clk_name);
 		ret = -ENOENT;
-		goto failed_get_clk;
+		goto failed;
 	}
 	clk_prepare_enable(ctrl->clk);
 
@@ -551,21 +551,8 @@ failed_path_init:
 		path_deinit(path_plat);
 	}
 
-	if (ctrl->clk) {
-		devm_clk_put(ctrl->dev, ctrl->clk);
-		clk_disable_unprepare(ctrl->clk);
-	}
-failed_get_clk:
-	devm_free_irq(ctrl->dev, ctrl->irq, ctrl);
+	clk_disable_unprepare(ctrl->clk);
 failed:
-	if (ctrl) {
-		if (ctrl->reg_base)
-			devm_iounmap(ctrl->dev, ctrl->reg_base);
-		devm_release_mem_region(ctrl->dev, res->start,
-				resource_size(res));
-		devm_kfree(ctrl->dev, ctrl);
-	}
-
 	dev_err(&pdev->dev, "device init failed\n");
 
 	return ret;
-- 
1.8.4.rc3


^ permalink raw reply related

* Re: [PATCH v3 1/2] video: ARM CLCD: Add DT support
From: Russell King - ARM Linux @ 2013-09-23 16:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52406646.709@wwwdotorg.org>

On Mon, Sep 23, 2013 at 10:03:18AM -0600, Stephen Warren wrote:
> It sounds like you could just put LCDControl & 0x2e in the DT rather
> than using values such as 0x100..0x107, which don't appear to match the
> register format you mentioned above.

No.  Platforms which route the outputs to something like VGA or HDMI can
change the framebuffer format.  Your suggestions is far too restrictive.

^ permalink raw reply

* Re: [PATCH v3 1/2] video: ARM CLCD: Add DT support
From: Stephen Warren @ 2013-09-23 16:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1379522363.9244.12.camel@hornet>

On 09/18/2013 10:39 AM, Pawel Moll wrote:
> On Tue, 2013-09-17 at 22:34 +0100, Stephen Warren wrote:
>>> On Tue, 2013-09-17 at 17:36 +0100, Pawel Moll wrote:
>>>> On Mon, 2013-09-16 at 20:52 +0100, Stephen Warren wrote:
>>>>>> +- arm,pl11x,panel-data-pads: array of 24 cells, each of them describing
>>>>>> +				a function of one of the CLD pads,
>>>>>> +				starting from 0 up to 23; each pad can
>>>>>> +				be described by one of the following values:
>>>>>> +	- 0: reserved (not connected)
>>>>>> +	- 0x100-0x107: color upper STN panel data 0 to 7
>>>>> ...
>>>>>
>>>>> I assume those are the raw values that go into the HW?
>>>
>>> No, they can be considered "labels", defining the way pads are used
>>> (wired). Then, basing on this information, the driver is configuring the
>>> cell, eg. selecting STN or TFT mode (thus switching the output between
>>> panel formatter and raw RGB data source).
>>
>> Oh, why not just use the raw values from the HW registers for this?
> 
> How do you mean? Based on the the way the "LCD data" lines are wired up,
> the driver has to decide whether to select STN (LCDControl &= ~(1<<5))
> or TFT mode (LCDControl |= 1<<5), then figures out what memory pixel
> formats are possible (based on this will set LCDControl[3..1] in
> runtime, depending on the mode selected by the user). There isn't a
> separate register as such configuring output pads. It's just the way
> they can used depends on the way they're wired up.

It sounds like you could just put LCDControl & 0x2e in the DT rather
than using values such as 0x100..0x107, which don't appear to match the
register format you mentioned above.


^ permalink raw reply

* Re: [PATCH 0/7] driver core: prevent deferred probe with platform_driver_probe
From: Sascha Hauer @ 2013-09-23 15:24 UTC (permalink / raw)
  To: Johan Hovold
  Cc: linux-fbdev, linux-usb, Mark Brown, Greg Kroah-Hartman,
	linux-pcmcia, linux-mmc, linux-kernel, linux-mtd, Grant Likely
In-Reply-To: <20130923152018.GB1454@localhost>

On Mon, Sep 23, 2013 at 05:20:18PM +0200, Johan Hovold wrote:
> On Mon, Sep 23, 2013 at 04:50:29PM +0200, Sascha Hauer wrote:
> > On Mon, Sep 23, 2013 at 04:27:25PM +0200, Johan Hovold wrote:
> > > Deferred probing cannot be used with platform_driver_probe as by the
> > > time probing is retried either the driver has been unregistered or its
> > > probe function has been set to platform_drv_probe_fail.
> > > 
> > > With commit e9354576 ("gpiolib: Defer failed gpio requests by default")
> > > the gpio subsystem started returning -EPROBE_DEFER, which in turn
> > > several platform drivers using platform_driver_probe return to driver
> > > core. Other subsystems (e.g. regulator) has since started doing the
> > > same.
> > > 
> > > The first patch in this series prevents platform drivers using
> > > platform_driver_probe from requesting probe deferral while warning that
> > > it is not supported.
> > > 
> > > The remaining patches move six platform-driver probe functions that rely
> > > on gpio_request out of __init. There are likely other probe functions
> > > that might return -EPROBE_DEFER and should be moved out of __init as
> > > well.
> > 
> > As usually when I read this I wonder why platform_driver_probe exists
> > anyway. The only advantage I can think off is that the probe functions
> > are in __init and thus can be disposed of later. Now you remove the
> > __init annotations from these probe functions. Wouldn't it be better to
> > convert the drivers to regular platform_driver_register instead?
> 
> Perhaps that paragraph was a bit unclear: I move them out of __init
> _and_ use platform_driver_register instead of platform_driver_probe as
> well.

Oh yes, I should have looked closer. Sorry for the noise.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH 0/7] driver core: prevent deferred probe with platform_driver_probe
From: Johan Hovold @ 2013-09-23 15:20 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Johan Hovold, Greg Kroah-Hartman, Grant Likely, Mark Brown,
	linux-kernel, linux-mmc, linux-mtd, linux-pcmcia, linux-usb,
	linux-fbdev
In-Reply-To: <20130923145029.GV30088@pengutronix.de>

On Mon, Sep 23, 2013 at 04:50:29PM +0200, Sascha Hauer wrote:
> On Mon, Sep 23, 2013 at 04:27:25PM +0200, Johan Hovold wrote:
> > Deferred probing cannot be used with platform_driver_probe as by the
> > time probing is retried either the driver has been unregistered or its
> > probe function has been set to platform_drv_probe_fail.
> > 
> > With commit e9354576 ("gpiolib: Defer failed gpio requests by default")
> > the gpio subsystem started returning -EPROBE_DEFER, which in turn
> > several platform drivers using platform_driver_probe return to driver
> > core. Other subsystems (e.g. regulator) has since started doing the
> > same.
> > 
> > The first patch in this series prevents platform drivers using
> > platform_driver_probe from requesting probe deferral while warning that
> > it is not supported.
> > 
> > The remaining patches move six platform-driver probe functions that rely
> > on gpio_request out of __init. There are likely other probe functions
> > that might return -EPROBE_DEFER and should be moved out of __init as
> > well.
> 
> As usually when I read this I wonder why platform_driver_probe exists
> anyway. The only advantage I can think off is that the probe functions
> are in __init and thus can be disposed of later. Now you remove the
> __init annotations from these probe functions. Wouldn't it be better to
> convert the drivers to regular platform_driver_register instead?

Perhaps that paragraph was a bit unclear: I move them out of __init
_and_ use platform_driver_register instead of platform_driver_probe as
well.

Johan

^ permalink raw reply

* [PATCH] neofb: fix error return code in neofb_probe()
From: Wei Yongjun @ 2013-09-23 15:00 UTC (permalink / raw)
  To: linux-fbdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Fix to return a negative error code from the error handling
case instead of 0, as done elsewhere in this function.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/video/neofb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/video/neofb.c b/drivers/video/neofb.c
index 891b0bf..2a3e5bd 100644
--- a/drivers/video/neofb.c
+++ b/drivers/video/neofb.c
@@ -2075,6 +2075,7 @@ static int neofb_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (!fb_find_mode(&info->var, info, mode_option, NULL, 0,
 			info->monspecs.modedb, 16)) {
 		printk(KERN_ERR "neofb: Unable to find usable video mode.\n");
+		err = -EINVAL;
 		goto err_map_video;
 	}
 
@@ -2097,7 +2098,8 @@ static int neofb_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	       info->fix.smem_len >> 10, info->var.xres,
 	       info->var.yres, h_sync / 1000, h_sync % 1000, v_sync);
 
-	if (fb_alloc_cmap(&info->cmap, 256, 0) < 0)
+	err = fb_alloc_cmap(&info->cmap, 256, 0);
+	if (err < 0)
 		goto err_map_video;
 
 	err = register_framebuffer(info);


^ permalink raw reply related

* [PATCH] s3fb: fix error return code in s3_pci_probe()
From: Wei Yongjun @ 2013-09-23 15:00 UTC (permalink / raw)
  To: linux-fbdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Fix to return -EINVAL when virtual vertical size smaller than real
instead of 0, as done elsewhere in this function. Also remove dup code.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/video/s3fb.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/video/s3fb.c b/drivers/video/s3fb.c
index 47ca86c..d838ba8 100644
--- a/drivers/video/s3fb.c
+++ b/drivers/video/s3fb.c
@@ -1336,14 +1336,7 @@ static int s3_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 			(info->var.bits_per_pixel * info->var.xres_virtual);
 	if (info->var.yres_virtual < info->var.yres) {
 		dev_err(info->device, "virtual vertical size smaller than real\n");
-		goto err_find_mode;
-	}
-
-	/* maximize virtual vertical size for fast scrolling */
-	info->var.yres_virtual = info->fix.smem_len * 8 /
-			(info->var.bits_per_pixel * info->var.xres_virtual);
-	if (info->var.yres_virtual < info->var.yres) {
-		dev_err(info->device, "virtual vertical size smaller than real\n");
+		rc = -EINVAL;
 		goto err_find_mode;
 	}
 


^ permalink raw reply related

* Re: [PATCH 0/7] driver core: prevent deferred probe with platform_driver_probe
From: Sascha Hauer @ 2013-09-23 14:50 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Greg Kroah-Hartman, Grant Likely, Mark Brown, linux-kernel,
	linux-mmc, linux-mtd, linux-pcmcia, linux-usb, linux-fbdev
In-Reply-To: <1379946452-25649-1-git-send-email-jhovold@gmail.com>

On Mon, Sep 23, 2013 at 04:27:25PM +0200, Johan Hovold wrote:
> Deferred probing cannot be used with platform_driver_probe as by the
> time probing is retried either the driver has been unregistered or its
> probe function has been set to platform_drv_probe_fail.
> 
> With commit e9354576 ("gpiolib: Defer failed gpio requests by default")
> the gpio subsystem started returning -EPROBE_DEFER, which in turn
> several platform drivers using platform_driver_probe return to driver
> core. Other subsystems (e.g. regulator) has since started doing the
> same.
> 
> The first patch in this series prevents platform drivers using
> platform_driver_probe from requesting probe deferral while warning that
> it is not supported.
> 
> The remaining patches move six platform-driver probe functions that rely
> on gpio_request out of __init. There are likely other probe functions
> that might return -EPROBE_DEFER and should be moved out of __init as
> well.

As usually when I read this I wonder why platform_driver_probe exists
anyway. The only advantage I can think off is that the probe functions
are in __init and thus can be disposed of later. Now you remove the
__init annotations from these probe functions. Wouldn't it be better to
convert the drivers to regular platform_driver_register instead?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* [PATCH 7/7] backlight: atmel-pwm-bl: fix deferred probe from __init
From: Johan Hovold @ 2013-09-23 14:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Grant Likely, Mark Brown, linux-kernel, linux-mmc, linux-mtd,
	linux-pcmcia, linux-usb, linux-fbdev, Johan Hovold,
	Richard Purdie, Jingoo Han, Jean-Christophe Plagniol-Villard
In-Reply-To: <1379946452-25649-1-git-send-email-jhovold@gmail.com>

Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.

Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.

Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 0393d82..f7447f7 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -118,7 +118,7 @@ static const struct backlight_ops atmel_pwm_bl_ops = {
 	.update_status  = atmel_pwm_bl_set_intensity,
 };
 
-static int __init atmel_pwm_bl_probe(struct platform_device *pdev)
+static int atmel_pwm_bl_probe(struct platform_device *pdev)
 {
 	struct backlight_properties props;
 	const struct atmel_pwm_bl_platform_data *pdata;
@@ -202,7 +202,7 @@ err_free_mem:
 	return retval;
 }
 
-static int __exit atmel_pwm_bl_remove(struct platform_device *pdev)
+static int atmel_pwm_bl_remove(struct platform_device *pdev)
 {
 	struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
 
@@ -220,10 +220,11 @@ static struct platform_driver atmel_pwm_bl_driver = {
 		.name = "atmel-pwm-bl",
 	},
 	/* REVISIT add suspend() and resume() */
-	.remove = __exit_p(atmel_pwm_bl_remove),
+	.probe = atmel_pwm_bl_probe,
+	.remove = atmel_pwm_bl_remove,
 };
 
-module_platform_driver_probe(atmel_pwm_bl_driver, atmel_pwm_bl_probe);
+module_platform_driver(atmel_pwm_bl_driver);
 
 MODULE_AUTHOR("Hans-Christian egtvedt <hans-christian.egtvedt@atmel.com>");
 MODULE_DESCRIPTION("Atmel PWM backlight driver");
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 6/7] usb: phy: gpio-vbus: fix deferred probe from __init
From: Johan Hovold @ 2013-09-23 14:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-fbdev, linux-usb, Mark Brown, Johan Hovold, linux-pcmcia,
	linux-mmc, linux-kernel, Felipe Balbi, linux-mtd, Grant Likely
In-Reply-To: <1379946452-25649-1-git-send-email-jhovold@gmail.com>

Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.

Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
and 04bf3011 ("regulator: Support driver probe deferral") this driver
might return -EPROBE_DEFER if a gpio_request or regulator_get fails.

Cc: Felipe Balbi <balbi@ti.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/usb/phy/phy-gpio-vbus-usb.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/phy/phy-gpio-vbus-usb.c b/drivers/usb/phy/phy-gpio-vbus-usb.c
index b2f29c9..02799a5 100644
--- a/drivers/usb/phy/phy-gpio-vbus-usb.c
+++ b/drivers/usb/phy/phy-gpio-vbus-usb.c
@@ -241,7 +241,7 @@ static int gpio_vbus_set_suspend(struct usb_phy *phy, int suspend)
 
 /* platform driver interface */
 
-static int __init gpio_vbus_probe(struct platform_device *pdev)
+static int gpio_vbus_probe(struct platform_device *pdev)
 {
 	struct gpio_vbus_mach_info *pdata = dev_get_platdata(&pdev->dev);
 	struct gpio_vbus_data *gpio_vbus;
@@ -349,7 +349,7 @@ err_gpio:
 	return err;
 }
 
-static int __exit gpio_vbus_remove(struct platform_device *pdev)
+static int gpio_vbus_remove(struct platform_device *pdev)
 {
 	struct gpio_vbus_data *gpio_vbus = platform_get_drvdata(pdev);
 	struct gpio_vbus_mach_info *pdata = dev_get_platdata(&pdev->dev);
@@ -398,8 +398,6 @@ static const struct dev_pm_ops gpio_vbus_dev_pm_ops = {
 };
 #endif
 
-/* NOTE:  the gpio-vbus device may *NOT* be hotplugged */
-
 MODULE_ALIAS("platform:gpio-vbus");
 
 static struct platform_driver gpio_vbus_driver = {
@@ -410,10 +408,11 @@ static struct platform_driver gpio_vbus_driver = {
 		.pm = &gpio_vbus_dev_pm_ops,
 #endif
 	},
-	.remove  = __exit_p(gpio_vbus_remove),
+	.probe		= gpio_vbus_probe,
+	.remove		= gpio_vbus_remove,
 };
 
-module_platform_driver_probe(gpio_vbus_driver, gpio_vbus_probe);
+module_platform_driver(gpio_vbus_driver);
 
 MODULE_DESCRIPTION("simple GPIO controlled OTG transceiver driver");
 MODULE_AUTHOR("Philipp Zabel");
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 5/7] usb: gadget: pxa25x_udc: fix deferred probe from __init
From: Johan Hovold @ 2013-09-23 14:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Grant Likely, Mark Brown, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-pcmcia-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Johan Hovold, Eric Miao,
	Russell King, Haojian Zhuang, Felipe Balbi
In-Reply-To: <1379946452-25649-1-git-send-email-jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.

Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.

Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Felipe Balbi <balbi@ti.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/usb/gadget/pxa25x_udc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/pxa25x_udc.c b/drivers/usb/gadget/pxa25x_udc.c
index cc92074..0ac6064 100644
--- a/drivers/usb/gadget/pxa25x_udc.c
+++ b/drivers/usb/gadget/pxa25x_udc.c
@@ -2054,7 +2054,7 @@ static struct pxa25x_udc memory = {
 /*
  *	probe - binds to the platform device
  */
-static int __init pxa25x_udc_probe(struct platform_device *pdev)
+static int pxa25x_udc_probe(struct platform_device *pdev)
 {
 	struct pxa25x_udc *dev = &memory;
 	int retval, irq;
@@ -2203,7 +2203,7 @@ static void pxa25x_udc_shutdown(struct platform_device *_dev)
 	pullup_off();
 }
 
-static int __exit pxa25x_udc_remove(struct platform_device *pdev)
+static int pxa25x_udc_remove(struct platform_device *pdev)
 {
 	struct pxa25x_udc *dev = platform_get_drvdata(pdev);
 
@@ -2294,7 +2294,8 @@ static int pxa25x_udc_resume(struct platform_device *dev)
 
 static struct platform_driver udc_driver = {
 	.shutdown	= pxa25x_udc_shutdown,
-	.remove		= __exit_p(pxa25x_udc_remove),
+	.probe		= pxa25x_udc_probe,
+	.remove		= pxa25x_udc_remove,
 	.suspend	= pxa25x_udc_suspend,
 	.resume		= pxa25x_udc_resume,
 	.driver		= {
@@ -2303,7 +2304,7 @@ static struct platform_driver udc_driver = {
 	},
 };
 
-module_platform_driver_probe(udc_driver, pxa25x_udc_probe);
+module_platform_driver(udc_driver);
 
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR("Frank Becker, Robert Schwebel, David Brownell");
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 4/7] pcmcia: at91_cf: fix deferred probe from __init
From: Johan Hovold @ 2013-09-23 14:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-fbdev, linux-usb, Mark Brown, Nicolas Ferre, Johan Hovold,
	linux-pcmcia, linux-mmc, linux-kernel, linux-mtd, Grant Likely,
	Jean-Christophe PLAGNIOL-VILLARD
In-Reply-To: <1379946452-25649-1-git-send-email-jhovold@gmail.com>

Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.

Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.

Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/pcmcia/at91_cf.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/pcmcia/at91_cf.c b/drivers/pcmcia/at91_cf.c
index b8f5acf..de24232 100644
--- a/drivers/pcmcia/at91_cf.c
+++ b/drivers/pcmcia/at91_cf.c
@@ -245,7 +245,7 @@ static int at91_cf_dt_init(struct platform_device *pdev)
 }
 #endif
 
-static int __init at91_cf_probe(struct platform_device *pdev)
+static int at91_cf_probe(struct platform_device *pdev)
 {
 	struct at91_cf_socket	*cf;
 	struct at91_cf_data	*board = pdev->dev.platform_data;
@@ -354,7 +354,7 @@ fail0a:
 	return status;
 }
 
-static int __exit at91_cf_remove(struct platform_device *pdev)
+static int at91_cf_remove(struct platform_device *pdev)
 {
 	struct at91_cf_socket	*cf = platform_get_drvdata(pdev);
 
@@ -404,14 +404,13 @@ static struct platform_driver at91_cf_driver = {
 		.owner		= THIS_MODULE,
 		.of_match_table = of_match_ptr(at91_cf_dt_ids),
 	},
-	.remove		= __exit_p(at91_cf_remove),
+	.probe		= at91_cf_probe,
+	.remove		= at91_cf_remove,
 	.suspend	= at91_cf_suspend,
 	.resume		= at91_cf_resume,
 };
 
-/*--------------------------------------------------------------------------*/
-
-module_platform_driver_probe(at91_cf_driver, at91_cf_probe);
+module_platform_driver(at91_cf_driver);
 
 MODULE_DESCRIPTION("AT91 Compact Flash Driver");
 MODULE_AUTHOR("David Brownell");
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 3/7] mtd: atmel_nand: fix deferred probe from __init
From: Johan Hovold @ 2013-09-23 14:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Grant Likely, Mark Brown, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-pcmcia-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Johan Hovold, David Woodhouse,
	Josh Wu
In-Reply-To: <1379946452-25649-1-git-send-email-jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.

Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if a gpio_request fails.

Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Josh Wu <josh.wu@atmel.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/mtd/nand/atmel_nand.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 060feea..bd1ce7d 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -1139,7 +1139,7 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
 	return 0;
 }
 
-static int __init atmel_pmecc_nand_init_params(struct platform_device *pdev,
+static int atmel_pmecc_nand_init_params(struct platform_device *pdev,
 					 struct atmel_nand_host *host)
 {
 	struct mtd_info *mtd = &host->mtd;
@@ -1548,7 +1548,7 @@ static int atmel_of_init_port(struct atmel_nand_host *host,
 }
 #endif
 
-static int __init atmel_hw_nand_init_params(struct platform_device *pdev,
+static int atmel_hw_nand_init_params(struct platform_device *pdev,
 					 struct atmel_nand_host *host)
 {
 	struct mtd_info *mtd = &host->mtd;
@@ -1987,7 +1987,7 @@ static struct platform_driver atmel_nand_nfc_driver;
 /*
  * Probe for the NAND device.
  */
-static int __init atmel_nand_probe(struct platform_device *pdev)
+static int atmel_nand_probe(struct platform_device *pdev)
 {
 	struct atmel_nand_host *host;
 	struct mtd_info *mtd;
@@ -2184,7 +2184,7 @@ err_nand_ioremap:
 /*
  * Remove a NAND device.
  */
-static int __exit atmel_nand_remove(struct platform_device *pdev)
+static int atmel_nand_remove(struct platform_device *pdev)
 {
 	struct atmel_nand_host *host = platform_get_drvdata(pdev);
 	struct mtd_info *mtd = &host->mtd;
@@ -2270,7 +2270,8 @@ static struct platform_driver atmel_nand_nfc_driver = {
 };
 
 static struct platform_driver atmel_nand_driver = {
-	.remove		= __exit_p(atmel_nand_remove),
+	.probe		= atmel_nand_probe,
+	.remove		= atmel_nand_remove,
 	.driver		= {
 		.name	= "atmel_nand",
 		.owner	= THIS_MODULE,
@@ -2278,7 +2279,7 @@ static struct platform_driver atmel_nand_driver = {
 	},
 };
 
-module_platform_driver_probe(atmel_nand_driver, atmel_nand_probe);
+module_platform_driver(atmel_nand_driver);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Rick Bronson");
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 2/7] mmc: mvsdio: fix deferred probe from __init
From: Johan Hovold @ 2013-09-23 14:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Grant Likely, Mark Brown, linux-kernel, linux-mmc, linux-mtd,
	linux-pcmcia, linux-usb, linux-fbdev, Johan Hovold, Nicolas Pitre,
	Chris Ball
In-Reply-To: <1379946452-25649-1-git-send-email-jhovold@gmail.com>

Move probe out of __init section and don't use platform_driver_probe
which cannot be used with deferred probing.

Since commit e9354576 ("gpiolib: Defer failed gpio requests by default")
this driver might return -EPROBE_DEFER if the mmc_gpio_request_cd fails.

Cc: Nicolas Pitre <nico@fluxnic.net>
Cc: Chris Ball <cjb@laptop.org>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/mmc/host/mvsdio.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index 06c5b0b..deecee0 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -655,7 +655,7 @@ static const struct mmc_host_ops mvsd_ops = {
 	.enable_sdio_irq	= mvsd_enable_sdio_irq,
 };
 
-static void __init
+static void
 mv_conf_mbus_windows(struct mvsd_host *host,
 		     const struct mbus_dram_target_info *dram)
 {
@@ -677,7 +677,7 @@ mv_conf_mbus_windows(struct mvsd_host *host,
 	}
 }
 
-static int __init mvsd_probe(struct platform_device *pdev)
+static int mvsd_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct mmc_host *mmc = NULL;
@@ -819,7 +819,7 @@ out:
 	return ret;
 }
 
-static int __exit mvsd_remove(struct platform_device *pdev)
+static int mvsd_remove(struct platform_device *pdev)
 {
 	struct mmc_host *mmc = platform_get_drvdata(pdev);
 
@@ -872,7 +872,8 @@ static const struct of_device_id mvsdio_dt_ids[] = {
 MODULE_DEVICE_TABLE(of, mvsdio_dt_ids);
 
 static struct platform_driver mvsd_driver = {
-	.remove		= __exit_p(mvsd_remove),
+	.probe		= mvsd_probe,
+	.remove		= mvsd_remove,
 	.suspend	= mvsd_suspend,
 	.resume		= mvsd_resume,
 	.driver		= {
@@ -881,7 +882,7 @@ static struct platform_driver mvsd_driver = {
 	},
 };
 
-module_platform_driver_probe(mvsd_driver, mvsd_probe);
+module_platform_driver(mvsd_driver);
 
 /* maximum card clock frequency (default 50MHz) */
 module_param(maxfreq, int, 0);
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 1/7] driver core: prevent deferred probe with platform_driver_probe
From: Johan Hovold @ 2013-09-23 14:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-fbdev, linux-usb, Mark Brown, Johan Hovold, linux-pcmcia,
	linux-mmc, linux-kernel, linux-mtd, Grant Likely
In-Reply-To: <1379946452-25649-1-git-send-email-jhovold@gmail.com>

Prevent drivers relying on platform_driver_probe from requesting
deferred probing in order to avoid further futile probe attempts (either
the driver has been unregistered or its probe function has been set to
platform_drv_probe_fail when probing is retried).

Note that several platform drivers currently return subsystem errors
from probe and that these can include -EPROBE_DEFER (e.g. if a gpio
request fails).

Add a warning to platform_drv_probe that can be used to catch drivers
that inadvertently request probe deferral while using
platform_driver_probe.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/base/platform.c         | 17 +++++++++++++----
 include/linux/platform_device.h |  1 +
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 4f8bef3..47051cd 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -488,6 +488,11 @@ static int platform_drv_probe(struct device *_dev)
 	if (ret && ACPI_HANDLE(_dev))
 		acpi_dev_pm_detach(_dev, true);
 
+	if (drv->prevent_deferred_probe && ret = -EPROBE_DEFER) {
+		dev_warn(_dev, "probe deferral not supported\n");
+		ret = -ENXIO;
+	}
+
 	return ret;
 }
 
@@ -553,8 +558,7 @@ EXPORT_SYMBOL_GPL(platform_driver_unregister);
 /**
  * platform_driver_probe - register driver for non-hotpluggable device
  * @drv: platform driver structure
- * @probe: the driver probe routine, probably from an __init section,
- *         must not return -EPROBE_DEFER.
+ * @probe: the driver probe routine, probably from an __init section
  *
  * Use this instead of platform_driver_register() when you know the device
  * is not hotpluggable and has already been registered, and you want to
@@ -565,8 +569,7 @@ EXPORT_SYMBOL_GPL(platform_driver_unregister);
  * into system-on-chip processors, where the controller devices have been
  * configured as part of board setup.
  *
- * This is incompatible with deferred probing so probe() must not
- * return -EPROBE_DEFER.
+ * Note that this is incompatible with deferred probing.
  *
  * Returns zero if the driver registered and bound to a device, else returns
  * a negative error code and with the driver not registered.
@@ -576,6 +579,12 @@ int __init_or_module platform_driver_probe(struct platform_driver *drv,
 {
 	int retval, code;
 
+	/*
+	 * Prevent driver from requesting probe deferral to avoid further
+	 * futile probe attempts.
+	 */
+	drv->prevent_deferred_probe = true;
+
 	/* make sure driver won't have bind/unbind attributes */
 	drv->driver.suppress_bind_attrs = true;
 
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index ce8e4ff..16f6654 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -178,6 +178,7 @@ struct platform_driver {
 	int (*resume)(struct platform_device *);
 	struct device_driver driver;
 	const struct platform_device_id *id_table;
+	bool prevent_deferred_probe;
 };
 
 #define to_platform_driver(drv)	(container_of((drv), struct platform_driver, \
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 0/7] driver core: prevent deferred probe with platform_driver_probe
From: Johan Hovold @ 2013-09-23 14:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-fbdev, linux-usb, Mark Brown, Johan Hovold, linux-pcmcia,
	linux-mmc, linux-kernel, linux-mtd, Grant Likely
In-Reply-To: <20130923134028.GF21013@sirena.org.uk>

Deferred probing cannot be used with platform_driver_probe as by the
time probing is retried either the driver has been unregistered or its
probe function has been set to platform_drv_probe_fail.

With commit e9354576 ("gpiolib: Defer failed gpio requests by default")
the gpio subsystem started returning -EPROBE_DEFER, which in turn
several platform drivers using platform_driver_probe return to driver
core. Other subsystems (e.g. regulator) has since started doing the
same.

The first patch in this series prevents platform drivers using
platform_driver_probe from requesting probe deferral while warning that
it is not supported.

The remaining patches move six platform-driver probe functions that rely
on gpio_request out of __init. There are likely other probe functions
that might return -EPROBE_DEFER and should be moved out of __init as
well.

Note that the mvsdio, at91_cf and pxa25x_udc patches are completely
untested.

Johan


Johan Hovold (7):
  driver core: prevent deferred probe with platform_driver_probe
  mmc: mvsdio: fix deferred probe from __init
  mtd: atmel_nand: fix deferred probe from __init
  pcmcia: at91_cf: fix deferred probe from __init
  usb: gadget: pxa25x_udc: fix deferred probe from __init
  usb: phy: gpio-vbus: fix deferred probe from __init
  backlight: atmel-pwm-bl: fix deferred probe from __init

 drivers/base/platform.c                | 17 +++++++++++++----
 drivers/mmc/host/mvsdio.c              | 11 ++++++-----
 drivers/mtd/nand/atmel_nand.c          | 13 +++++++------
 drivers/pcmcia/at91_cf.c               | 11 +++++------
 drivers/usb/gadget/pxa25x_udc.c        |  9 +++++----
 drivers/usb/phy/phy-gpio-vbus-usb.c    | 11 +++++------
 drivers/video/backlight/atmel-pwm-bl.c |  9 +++++----
 include/linux/platform_device.h        |  1 +
 8 files changed, 47 insertions(+), 35 deletions(-)

-- 
1.8.3.2


^ permalink raw reply

* Re: [PATCH 37/51] DMA-API: usb: use new dma_coerce_mask_and_coherent()
From: Nicolas Ferre @ 2013-09-23 12:34 UTC (permalink / raw)
  To: Russell King, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	e1000-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-ide-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Solarflare linux maintainers,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b
  Cc: Alexander Shishkin, Greg Kroah-Hartman, Felipe Balbi, Kukjin Kim,
	Alan Stern, Tony Prisk, Stephen Warren
In-Reply-To: <E1VMmIV-0007jw-Gq-eh5Bv4kxaXIANfyc6IWni62ZND6+EDdj@public.gmane.org>

On 20/09/2013 00:02, Russell King :
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>   drivers/usb/chipidea/ci_hdrc_imx.c |    4 +---
>   drivers/usb/dwc3/dwc3-exynos.c     |    4 +---
>   drivers/usb/host/ehci-atmel.c      |    4 +---

For Atmel driver:

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

[..]

> diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
> index 5831a88..8e7323e 100644
> --- a/drivers/usb/host/ehci-atmel.c
> +++ b/drivers/usb/host/ehci-atmel.c
> @@ -90,9 +90,7 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
>   	 * Since shared usb code relies on it, set it here for now.
>   	 * Once we have dma capability bindings this can go away.
>   	 */
> -	if (!pdev->dev.dma_mask)
> -		pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> -	retval = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> +	retval = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>   	if (retval)
>   		goto fail_create_hcd;
>

[..]

Thanks Russell,
-- 
Nicolas Ferre

^ permalink raw reply

* Re: [PATCH 36/51] DMA-API: usb: use dma_set_coherent_mask()
From: Nicolas Ferre @ 2013-09-23 12:30 UTC (permalink / raw)
  To: Russell King, alsa-devel, b43-dev, devel, devicetree, dri-devel,
	e1000-devel, linux-arm-kernel, linux-crypto, linux-doc,
	linux-fbdev, linux-ide, linux-media, linux-mmc, linux-nvme,
	linux-omap, linuxppc-dev, linux-samsung-soc, linux-scsi,
	linux-tegra, linux-usb, linux-wireless, netdev,
	Solarflare linux maintainers, uclinux-dist-devel
  Cc: Kukjin Kim, Stephen Warren, Alexander Shishkin,
	Greg Kroah-Hartman, Felipe Balbi, Alan Stern
In-Reply-To: <E1VMmHX-0007jq-Cj@rmk-PC.arm.linux.org.uk>

On 20/09/2013 00:01, Russell King :
> The correct way for a driver to specify the coherent DMA mask is
> not to directly access the field in the struct device, but to use
> dma_set_coherent_mask().  Only arch and bus code should access this
> member directly.
>
> Convert all direct write accesses to using the correct API.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>   drivers/usb/chipidea/ci_hdrc_imx.c |    5 +++--
>   drivers/usb/dwc3/dwc3-exynos.c     |    5 +++--
>   drivers/usb/gadget/lpc32xx_udc.c   |    4 +++-
>   drivers/usb/host/ehci-atmel.c      |    5 +++--

For Atmel driver:

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

[..]

> diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
> index 3b645ff..5831a88 100644
> --- a/drivers/usb/host/ehci-atmel.c
> +++ b/drivers/usb/host/ehci-atmel.c
> @@ -92,8 +92,9 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
>   	 */
>   	if (!pdev->dev.dma_mask)
>   		pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> -	if (!pdev->dev.coherent_dma_mask)
> -		pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> +	retval = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> +	if (retval)
> +		goto fail_create_hcd;
>
>   	hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
>   	if (!hcd) {

[..]

Thanks,
-- 
Nicolas Ferre

^ permalink raw reply

* Re: [alsa-devel] [PATCH 43/51] DMA-API: dma: edma.c: no need to explicitly initialize DMA masks
From: Russell King - ARM Linux @ 2013-09-23 11:37 UTC (permalink / raw)
  To: Vinod Koul
  Cc: alsa-devel, linux-doc, linux-mmc, linux-fbdev, linux-nvme,
	linux-ide, devel, linux-samsung-soc, linux-scsi, e1000-devel,
	b43-dev, linux-media, devicetree, dri-devel, linux-tegra,
	Dan Williams, linux-omap, linux-arm-kernel,
	Solarflare linux maintainers, netdev, linux-usb, linux-wireless,
	linux-crypto, uclinux-dist-devel, linuxppc-dev
In-Reply-To: <20130923102533.GI17188@intel.com>

On Mon, Sep 23, 2013 at 03:55:33PM +0530, Vinod Koul wrote:
> On Fri, Sep 20, 2013 at 12:15:39AM +0100, Russell King wrote:
> > register_platform_device_full() can setup the DMA mask provided the
> > appropriate member is set in struct platform_device_info.  So lets
> > make that be the case.  This avoids a direct reference to the DMA
> > masks by this driver.
> > 
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Acked-by: Vinod Koul <vinod.koul@intel.com>
> 
> This also brings me question that should we force the driver to use the
> dma_set_mask_and_coherent() API or they have below flexiblity too?

There's two issues here:
1. dma_set_mask_and_coherent() will only work if dev->dma_mask points at
   some storage for the mask.  This needs to have .dma_mask in the
   platform_device_info initialised.

2. Yes, this driver should also be calling the appropriate DMA mask setting
   functions in addition to having the mask initialized at device creation
   time.

Here's a replacement patch, though maybe it would be better to roll all
the additions of dma_set_mask_and_coherent() in drivers/dma into one
patch?  In other words, combine the addition of this with these two
patches:

	dma: pl330: add dma_set_mask_and_coherent() call
	dma: pl08x: add dma_set_mask_and_coherent() call

8<==From: Russell King <rmk+kernel@arm.linux.org.uk>
Subject: [PATCH] DMA-API: dma: edma.c: no need to explicitly initialize DMA
 masks

register_platform_device_full() can setup the DMA mask provided the
appropriate member is set in struct platform_device_info.  So lets
make that be the case.  This avoids a direct reference to the DMA
masks by this driver.

While here, add the dma_set_mask_and_coherent() call which the DMA API
requires DMA-using drivers to call.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/edma.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index ff50ff4..fd5e48c 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -631,6 +631,10 @@ static int edma_probe(struct platform_device *pdev)
 	struct edma_cc *ecc;
 	int ret;
 
+	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (ret)
+		return ret;
+
 	ecc = devm_kzalloc(&pdev->dev, sizeof(*ecc), GFP_KERNEL);
 	if (!ecc) {
 		dev_err(&pdev->dev, "Can't allocate controller\n");
@@ -702,11 +706,13 @@ static struct platform_device *pdev0, *pdev1;
 static const struct platform_device_info edma_dev_info0 = {
 	.name = "edma-dma-engine",
 	.id = 0,
+	.dma_mask = DMA_BIT_MASK(32),
 };
 
 static const struct platform_device_info edma_dev_info1 = {
 	.name = "edma-dma-engine",
 	.id = 1,
+	.dma_mask = DMA_BIT_MASK(32),
 };
 
 static int edma_init(void)
@@ -720,8 +726,6 @@ static int edma_init(void)
 			ret = PTR_ERR(pdev0);
 			goto out;
 		}
-		pdev0->dev.dma_mask = &pdev0->dev.coherent_dma_mask;
-		pdev0->dev.coherent_dma_mask = DMA_BIT_MASK(32);
 	}
 
 	if (EDMA_CTLRS = 2) {
@@ -731,8 +735,6 @@ static int edma_init(void)
 			platform_device_unregister(pdev0);
 			ret = PTR_ERR(pdev1);
 		}
-		pdev1->dev.dma_mask = &pdev1->dev.coherent_dma_mask;
-		pdev1->dev.coherent_dma_mask = DMA_BIT_MASK(32);
 	}
 
 out:
-- 
1.7.4.4



^ permalink raw reply related

* Re: [alsa-devel] [PATCH 24/51] DMA-API: dma: pl330: add dma_set_mask_and_coherent() call
From: Vinod Koul @ 2013-09-23 10:55 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: alsa-devel, linux-doc, linux-wireless, linux-fbdev, dri-devel,
	linux-ide, devel, linux-samsung-soc, linux-scsi, e1000-devel,
	b43-dev, linux-media, devicetree, linux-nvme, linux-tegra,
	Dan Williams, linux-omap, linux-arm-kernel,
	Solarflare linux maintainers, netdev, linux-usb, linux-mmc,
	linux-crypto, uclinux-dist-devel, linuxppc-dev
In-Reply-To: <20130921200000.GS25647@n2100.arm.linux.org.uk>

On Sat, Sep 21, 2013 at 09:00:00PM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 20, 2013 at 07:26:27PM +0200, Heiko Stübner wrote:
> > Am Donnerstag, 19. September 2013, 23:49:01 schrieb Russell King:
> > > The DMA API requires drivers to call the appropriate dma_set_mask()
> > > functions before doing any DMA mapping.  Add this required call to
> > > the AMBA PL08x driver.
> > 			^--- copy and paste error - should of course be PL330
> 
> Fixed, thanks.
with fixed changelog...

Acked-by: Vinod Koul <vinod.koul@intel.com>

~Vinod

-- 

^ permalink raw reply

* Re: [alsa-devel] [PATCH 43/51] DMA-API: dma: edma.c: no need to explicitly initialize DMA masks
From: Vinod Koul @ 2013-09-23 10:37 UTC (permalink / raw)
  To: Russell King
  Cc: alsa-devel, linux-doc, linux-mmc, linux-fbdev, linux-nvme,
	linux-ide, devel, linux-samsung-soc, linux-scsi, e1000-devel,
	b43-dev, linux-media, devicetree, dri-devel, linux-tegra,
	Dan Williams, linux-omap, linux-arm-kernel,
	Solarflare linux maintainers, netdev, linux-usb, linux-wireless,
	linux-crypto, uclinux-dist-devel, linuxppc-dev
In-Reply-To: <E1VMnRj-0007sg-1Z@rmk-PC.arm.linux.org.uk>

On Fri, Sep 20, 2013 at 12:15:39AM +0100, Russell King wrote:
> register_platform_device_full() can setup the DMA mask provided the
> appropriate member is set in struct platform_device_info.  So lets
> make that be the case.  This avoids a direct reference to the DMA
> masks by this driver.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: Vinod Koul <vinod.koul@intel.com>

This also brings me question that should we force the driver to use the
dma_set_mask_and_coherent() API or they have below flexiblity too?

~Vinod

> ---
>  drivers/dma/edma.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index ff50ff4..7f9fe30 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -702,11 +702,13 @@ static struct platform_device *pdev0, *pdev1;
>  static const struct platform_device_info edma_dev_info0 = {
>  	.name = "edma-dma-engine",
>  	.id = 0,
> +	.dma_mask = DMA_BIT_MASK(32),
>  };
>  
>  static const struct platform_device_info edma_dev_info1 = {
>  	.name = "edma-dma-engine",
>  	.id = 1,
> +	.dma_mask = DMA_BIT_MASK(32),
>  };


>  
>  static int edma_init(void)
> @@ -720,8 +722,6 @@ static int edma_init(void)
>  			ret = PTR_ERR(pdev0);
>  			goto out;
>  		}
> -		pdev0->dev.dma_mask = &pdev0->dev.coherent_dma_mask;
> -		pdev0->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>  	}
>  
>  	if (EDMA_CTLRS = 2) {
> @@ -731,8 +731,6 @@ static int edma_init(void)
>  			platform_device_unregister(pdev0);
>  			ret = PTR_ERR(pdev1);
>  		}
> -		pdev1->dev.dma_mask = &pdev1->dev.coherent_dma_mask;
> -		pdev1->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>  	}
>  
>  out:
> -- 
> 1.7.4.4
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 

^ permalink raw reply

* Re: [alsa-devel] [PATCH 23/51] DMA-API: dma: pl08x: add dma_set_mask_and_coherent() call
From: Vinod Koul @ 2013-09-23 10:24 UTC (permalink / raw)
  To: Russell King
  Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	e1000-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-ide-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Solarflare linux maintainers,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b, Dan Williams
In-Reply-To: <E1VMm4v-0007hz-RC-eh5Bv4kxaXIANfyc6IWni62ZND6+EDdj@public.gmane.org>

On Thu, Sep 19, 2013 at 10:48:01PM +0100, Russell King wrote:
> The DMA API requires drivers to call the appropriate dma_set_mask()
> functions before doing any DMA mapping.  Add this required call to
> the AMBA PL08x driver.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: Vinod Koul <vinod.koul@intel.com>

~Vinod
> ---
>  drivers/dma/amba-pl08x.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
> index fce46c5..e51a983 100644
> --- a/drivers/dma/amba-pl08x.c
> +++ b/drivers/dma/amba-pl08x.c
> @@ -2055,6 +2055,11 @@ static int pl08x_probe(struct amba_device *adev, const struct amba_id *id)
>  	if (ret)
>  		return ret;
>  
> +	/* Ensure that we can do DMA */
> +	ret = dma_set_mask_and_coherent(&adev->dev, DMA_BIT_MASK(32));
> +	if (ret)
> +		goto out_no_pl08x;
> +
>  	/* Create the driver state holder */
>  	pl08x = kzalloc(sizeof(*pl08x), GFP_KERNEL);
>  	if (!pl08x) {
> -- 
> 1.7.4.4
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox