Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH 03/15] OMAPDSS: fix DPI and SDI device ids
From: Tony Lindgren @ 2013-09-17 16:20 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1377783120-14001-4-git-send-email-tomi.valkeinen@ti.com>

* Tomi Valkeinen <tomi.valkeinen@ti.com> [130829 06:40]:
> The DPI and SDI platform devices are currently created with the ID of
> -1. The ID doesn't currently affect anything.
> 
> However, we have added regulator supply entries for "omapdss_dpi.0" and
> "omapdss_sdi.0" to the board files, although these supply entries are
> not yet used. As the ID used for the devices is -1, these regulator
> supply entries will not work.
> 
> To fix the issue, assign ID of 0 to the devices. In the future there may
> be more than one DPI or SDI output, so it makes sense to have a proper
> ID for them.

This should be safe to queue by you along with other DSS patches:

Acked-by: Tony Lindgren <tony@atomide.com>
 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  arch/arm/mach-omap2/display.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> index ff37be1..03a0516 100644
> --- a/arch/arm/mach-omap2/display.c
> +++ b/arch/arm/mach-omap2/display.c
> @@ -400,7 +400,7 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
>  
>  	/* Create devices for DPI and SDI */
>  
> -	pdev = create_simple_dss_pdev("omapdss_dpi", -1,
> +	pdev = create_simple_dss_pdev("omapdss_dpi", 0,
>  			board_data, sizeof(*board_data), dss_pdev);
>  	if (IS_ERR(pdev)) {
>  		pr_err("Could not build platform_device for omapdss_dpi\n");
> @@ -408,7 +408,7 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
>  	}
>  
>  	if (cpu_is_omap34xx()) {
> -		pdev = create_simple_dss_pdev("omapdss_sdi", -1,
> +		pdev = create_simple_dss_pdev("omapdss_sdi", 0,
>  				board_data, sizeof(*board_data), dss_pdev);
>  		if (IS_ERR(pdev)) {
>  			pr_err("Could not build platform_device for omapdss_sdi\n");
> -- 
> 1.8.1.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 1/2] video: ARM CLCD: Add DT support
From: Pawel Moll @ 2013-09-17 16:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <52376180.1090001@wwwdotorg.org>

On Mon, 2013-09-16 at 20:52 +0100, Stephen Warren wrote:
> I think the binding should either always use names as the key, or use
> indices in interrupts as the key. Hence, I'd word that more like:
> 
> - interrupt-names: either the single entry "combined" representing a
> 			combined interrupt output (CLCDINTR), or the
> 			four entries "mbe", "vcomp", "lnbu", "fuf"
> 			representing the individual CLCDMBEINTR,
> 			CLCDVCOMPINTR, CLCDLNBUINTR, CLCDFUFINTR
> 			interrupts.
> - interrupts: contains an interrupt specifier for each entry in
> 		 interrupt-names.

Cool, works for me.

> > +- 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?
> 
> > +				Example sets of values for standard
> > +				panel interfaces:
> > +	- PL110 single colour STN panel:
> > +			<0x107 0x106 0x105 0x104 0x103 0x102 0x101 0x100>,
> > +			<0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>;
> 
> The indentation of the introductory text seems a little odd. 

It follows the first paragraph of this property description. But I may
re-format all this stuff. Really looking forward some the formal
syntax :-)

> Do we really need so many examples?

They cover all standard cases. If I was to write a tree for a new
platform not knowing anything about CLCD, I think I'd appreciate this
and don't believe this extra kB or two is a problem. Do you?

> > +Optional properties:
> > +
> > +- arm,pl11x,framebuffer-base: a pair of two values, address and size,
> > +				defining the framebuffer to be used;
> > +				to be used only if it is *not*
> > +				part of normal memory, as described
> > +				in /memory node
> 
> If the framebuffer is part of /memory, what happens then? Is the address
> not fixed (so the HW isn't yet set up) and hence a driver should
> allocate it?

Yes, if it wants to display anything :-) And as this is a normal and
expected behaviour, I don't think it deserves a note in the
documentation. I'm open to any suggestions that would make the wording
above emphasize the "weirdness" of situations requiring the property.

Paweł



^ permalink raw reply

* Re: [PATCH v3 1/2] video: ARM CLCD: Add DT support
From: Pawel Moll @ 2013-09-17 16:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1379435799.9205.16.camel@hornet>

Uh, sorry, missed one...

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).

Paweł



^ permalink raw reply

* Re: [PATCH 02/11] omapdss: HDMI: create a HDMI PLL library
From: Mike Turquette @ 2013-09-17 19:40 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Archit Taneja, Linux OMAP Mailing List, linux-fbdev
In-Reply-To: <523828A7.8050204@ti.com>

On Tue, Sep 17, 2013 at 3:02 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 17/09/13 12:38, Mike Turquette wrote:
>> Quoting Archit Taneja (2013-09-17 00:06:28)
>>> HDMI PLL is a block common to DSS in OMAP4, OMAP5 and DRA7x. Move the
>>> existing PLL functions from ti_hdmi_4xxx_ip.c and hdmi.c to a separate file.
>>> These funcs are called directly from the hdmi driver rather than hdmi_ip_ops
>>> function pointer calls.
>>>
>>> Add the PLL library function declarations to ti_hdmi.h. These will be shared
>>> amongst the omap4/5 hdmi platform drivers. Remove the PLL function pointer ops
>>> from the ti_hdmi_ip_ops struct. These will be shared amongst the omap4/5 hdmi
>>> platform drivers and other libraries.
>>>
>>> Signed-off-by: Archit Taneja <archit@ti.com>
>>
>> Would be cool to see this convert to the common clock framework
>> implementation (include/linux/clk-provider.h). It appears that this PLL
>> only needs to support .enable, .disable and .recalc_rate callbacks at
>> first glance.
>
> We've got a (very) long term plan to use CCF for DSS. And, if I'm not
> mistaken, the PLL used for HDMI and DSI is the same as used elsewhere in
> OMAP (I don't remember the PLL type name).
>
> I think the main issue with DSS clocks is that we require as exact
> clocks as possible, and there are multiple dividers/multipliers on the
> clock path. So we iterate over the divider/multiplier ranges, trying to
> find as good freq match as possible.
>
> So if the clock path is composed from "black boxes", and we can only ask
> for a certain frequency, and get back probably some other frequency, it
> gets quite difficult to find good clock matches. Well, at least I
> imagine so, I have not tried to implement that.

I hope that the existing CLK_SET_RATE_PARENT flag could help you get
the frequency you need; it causes a call to clk_set_rate(leaf_clk,
target_rate) to walk up the chain of parents and configure rates as
needed.

However I have been looking at standardizing a way to define clock
rate tables, possibly in DT. In many cases it is a board-specific
issue (e.g. different oscillators or other reference clocks that feed
into the SoC) and specifying rate tables in DT is one way to store
known-good configurations for complex clock subtrees.

Regards,
Mike

>
>  Tomi
>
>

^ permalink raw reply

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

On 09/17/2013 10:51 AM, Pawel Moll wrote:
> Uh, sorry, missed one...
> 
> 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?

^ permalink raw reply

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

On 09/17/2013 10:36 AM, Pawel Moll wrote:
> On Mon, 2013-09-16 at 20:52 +0100, Stephen Warren wrote:
>> I think the binding should either always use names as the key, or use
>> indices in interrupts as the key. Hence, I'd word that more like:

>> Do we really need so many examples?
> 
> They cover all standard cases. If I was to write a tree for a new
> platform not knowing anything about CLCD, I think I'd appreciate this
> and don't believe this extra kB or two is a problem. Do you?

I guess it's not a problem. It's just unusual.

>>> +Optional properties:
>>> +
>>> +- arm,pl11x,framebuffer-base: a pair of two values, address and size,
>>> +				defining the framebuffer to be used;
>>> +				to be used only if it is *not*
>>> +				part of normal memory, as described
>>> +				in /memory node
>>
>> If the framebuffer is part of /memory, what happens then? Is the address
>> not fixed (so the HW isn't yet set up) and hence a driver should
>> allocate it?
> 
> Yes, if it wants to display anything :-) And as this is a normal and
> expected behaviour, I don't think it deserves a note in the
> documentation. I'm open to any suggestions that would make the wording
> above emphasize the "weirdness" of situations requiring the property.

Perhaps:

A pair of two values, address and size, defining the framebuffer to be
used. If not present, the framebuffer may be located anywhere in memory.

Or, is this intended to represent where the HW is already scanning out
from? If so, then perhaps:

A pair of two values, address and size, defining the location of the
framebuffer that the controller is currently configured to display. If
not present, the controller may or may not already be active.

(although presumably if the controller is already active, then the
address can simply be read out of the HW register, so there's no need
for a DT property).

^ permalink raw reply

* [PATCH] fbcon: fix deadlock in fbcon_generic_blank()
From: John Tapsell @ 2013-09-17 22:29 UTC (permalink / raw)
  To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen
  Cc: Peter Hurley, Dave Airlie, linux-fbdev, linux-kernel

Do not lock fb_info when calling sending the FB_EVENT_CONBLANK
event.

In fbmem.c, the semantics are that we acquire the lock_fb_info first,
and then console_lock.  However when fbcon.c fbcon_generic_blank() is
called, the console lock could already be held.  Locking fb_info can
thus cause a deadlock.

fbmem.c sends the FB_EVENT_BLANK without locking lock_fb_info first, so
this change introduces similar behaviour.

Signed-off-by: John Tapsell <johnflux@gmail.com>

---
 drivers/video/console/fbcon.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 6b4fb5c..8546441 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -2333,13 +2333,9 @@ static void fbcon_generic_blank(struct vc_data
*vc, struct fb_info *info,
         vc->vc_video_erase_char = oldc;
     }

-
-    if (!lock_fb_info(info))
-        return;
     event.info = info;
     event.data = &blank;
     fb_notifier_call_chain(FB_EVENT_CONBLANK, &event);
-    unlock_fb_info(info);
 }

 static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)
-- 
1.8.1.2

^ permalink raw reply related

* Re: [PATCH 02/11] omapdss: HDMI: create a HDMI PLL library
From: Tomi Valkeinen @ 2013-09-18  7:00 UTC (permalink / raw)
  To: Mike Turquette; +Cc: Archit Taneja, Linux OMAP Mailing List, linux-fbdev
In-Reply-To: <CAPtuhTh-6JJnpqp=0FCQqkPS2yBUmj7-zniY4vRQR9xq+t8htg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1797 bytes --]

On 17/09/13 22:40, Mike Turquette wrote:

> I hope that the existing CLK_SET_RATE_PARENT flag could help you get
> the frequency you need; it causes a call to clk_set_rate(leaf_clk,
> target_rate) to walk up the chain of parents and configure rates as
> needed.

Hmm, I'm not quite sure what that does, but it doesn't sound like it
would help. The only way I know to find good clocks is to iterate over
the dividers for each clock on the path.

And to complicate things, clocks from the "middle" of the path are used
for some purposes, so it's not only the final clock that we're
interested in.

For example, DSI: DSI PLL produces a high freq clock that goes to DSI
PHY. That one is used for the DSI bus clock. The high freq clock from
DSI PLL also goes to two dividers, of which one goes to DSI IP, and the
second goes to DISPC. DISPC clock is then divided with two dividers in
row, and the resulting clock goes also to DSI IP.

All those different clocks have restrictions and dependencies to each
other, like this one needs to be higher than that one, or ClkA = N *
ClkB, etc.

> However I have been looking at standardizing a way to define clock
> rate tables, possibly in DT. In many cases it is a board-specific
> issue (e.g. different oscillators or other reference clocks that feed
> into the SoC) and specifying rate tables in DT is one way to store
> known-good configurations for complex clock subtrees.

Well, for some boards, which only have an LCD, we could probably get the
clock from a table. But if the display used (say, external monitor)
supports multiple resolutions, such a table is not feasible. We could
have some clocks there in the table, but we'd still need a dynamic way
to figure out the dividers for other clock rates.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* [PATCH] radeon: Conditionally compile PM code
From: Thierry Reding @ 2013-09-18 12:48 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen
  Cc: linux-fbdev, linux-kernel

The power management code is only used on X86 and PowerMac. To prevent
the compiler from warning about unused code, only build when PM and one
of X86 or PowerMac is selected.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/video/aty/radeon_pm.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/video/aty/radeon_pm.c b/drivers/video/aty/radeon_pm.c
index f7091ec..f4317f1 100644
--- a/drivers/video/aty/radeon_pm.c
+++ b/drivers/video/aty/radeon_pm.c
@@ -1427,6 +1427,8 @@ static void radeon_pm_full_reset_sdram(struct radeonfb_info *rinfo)
 	mdelay( 15);
 }
 
+#if defined(CONFIG_PM)
+#if defined(CONFIG_X86) || defined(CONFIG_PPC_PMAC)
 static void radeon_pm_reset_pad_ctlr_strength(struct radeonfb_info *rinfo)
 {
 	u32 tmp, tmp2;
@@ -1939,9 +1941,10 @@ static void radeon_reinitialize_M10(struct radeonfb_info *rinfo)
 	 */
 	radeon_pm_m10_enable_lvds_spread_spectrum(rinfo);
 }
+#endif
 
 #ifdef CONFIG_PPC_OF
-
+#ifdef CONFIG_PPC_PMAC
 static void radeon_pm_m9p_reconfigure_mc(struct radeonfb_info *rinfo)
 {
 	OUTREG(MC_CNTL, rinfo->save_regs[46]);
@@ -2202,6 +2205,8 @@ static void radeon_reinitialize_M9P(struct radeonfb_info *rinfo)
 	radeon_pm_restore_pixel_pll(rinfo);
 	radeon_pm_m10_enable_lvds_spread_spectrum(rinfo);
 }
+#endif
+#endif
 
 #if 0 /* Not ready yet */
 static void radeon_reinitialize_QW(struct radeonfb_info *rinfo)
-- 
1.8.4


^ permalink raw reply related

* Re: [PATCH v3 1/2] video: ARM CLCD: Add DT support
From: Pawel Moll @ 2013-09-18 16:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5238CBE9.2060800@wwwdotorg.org>

On Tue, 2013-09-17 at 22:38 +0100, Stephen Warren wrote:
> >>> +Optional properties:
> >>> +
> >>> +- arm,pl11x,framebuffer-base: a pair of two values, address and size,
> >>> +				defining the framebuffer to be used;
> >>> +				to be used only if it is *not*
> >>> +				part of normal memory, as described
> >>> +				in /memory node
> >>
> >> If the framebuffer is part of /memory, what happens then? Is the address
> >> not fixed (so the HW isn't yet set up) and hence a driver should
> >> allocate it?
> > 
> > Yes, if it wants to display anything :-) And as this is a normal and
> > expected behaviour, I don't think it deserves a note in the
> > documentation. I'm open to any suggestions that would make the wording
> > above emphasize the "weirdness" of situations requiring the property.
> 
> Perhaps:
> 
> A pair of two values, address and size, defining the framebuffer to be
> used. If not present, the framebuffer may be located anywhere in memory.

Works with me, thanks!

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.

Paweł



^ permalink raw reply

* [PATCH] OMAPDSS: DISPC: set irq_safe for runtime PM
From: Tomi Valkeinen @ 2013-09-19 10:19 UTC (permalink / raw)
  To: linux-fbdev

We have a bug with omapdrm, where omapdrm calls dispc's pm_runtime
function in atomic context, and dispc's pm_runtime is not marked as
irq_safe:

BUG: sleeping function called from invalid context at drivers/base/power/runtime.c:952

Dispc's runtime PM callbacks are irq safe, so we can just set the
irq_safe flag to fix the issue.

However, in the long term, I'd rather have omapdrm manage the runtime PM
calls in a better way. Calling get/put for every small operation that
touches the dispc registers is very inefficient. It'd be better and
cleaner to have clear "in-use" and "not-in-use" states for dispc, so
that we don't need to do register context restore for small operations,
only to turn dispc off right afterwards.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dispc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 02a7340..4779750 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -3691,6 +3691,7 @@ static int __init omap_dispchw_probe(struct platform_device *pdev)
 	}
 
 	pm_runtime_enable(&pdev->dev);
+	pm_runtime_irq_safe(&pdev->dev);
 
 	r = dispc_runtime_get();
 	if (r)
-- 
1.8.1.2


^ permalink raw reply related

* Re: [PATCH] pwm-backlight: allow for non-increasing brightness levels
From: Thierry Reding @ 2013-09-19 11:56 UTC (permalink / raw)
  To: Mike Dunn
  Cc: Richard Purdie, Jingoo Han, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, Grant Likely, Rob Herring,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Robert Jarzmik, Marek Vasut
In-Reply-To: <1379010952-8928-1-git-send-email-mikedunn-kFrNdAxtuftBDgjK7y7TUQ@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2176 bytes --]

On Thu, Sep 12, 2013 at 11:35:52AM -0700, Mike Dunn wrote:
> Currently the driver assumes that the values specified in the brightness-levels
> device tree property increase as they are parsed from left to right.  But boards
> that invert the signal between the PWM output and the backlight will need to
> specify decreasing brightness-levels.  This patch removes the assumption that
> the last element of the array is the max value, and instead searches the array
> for the max value and uses that as the normalizing value when determining the
> duty cycle.

"maximum value", "... and uses that as the scale to normalize the duty
cycle"?

Also please wrap commit messages at 72 characters.

> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index 1fea627..d66aaa0 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -27,6 +27,7 @@ struct pwm_bl_data {
>  	unsigned int		period;
>  	unsigned int		lth_brightness;
>  	unsigned int		*levels;
> +	unsigned int		max_level;

Perhaps call this "scale"? Otherwise there some potential to mix it up
with max_brightness.

> @@ -195,7 +196,15 @@ static int pwm_backlight_probe(struct platform_device *pdev)
>  	}
>  
>  	if (data->levels) {
> -		max = data->levels[data->max_brightness];
> +		int i, max_value = 0, max_idx = 0;

i should be unsigned int to match the type of data->max_brightness.

> +		for (i = 0; i <= data->max_brightness; i++) {

There should be a blank line above this one to increase readability.

> +			if (data->levels[i] > max_value) {
> +				max_value = data->levels[i];
> +				max_idx = i;
> +			}
> +		}
> +		pb->max_level = max_idx;

Some here.

Also I suggest to just drop the max_ prefix from the local variables.
Perhaps also simplify all of it to something like:

	for (i = 0; i <= data->max_brightness; i++)
		if (data->levels[i] > pb->scale)
			pb->scale = data->levels[i];

And get rid of the index altogether. That way you can use pb->scale
directly during the computation of the duty cycle and don't have to
index the levels array over and over again.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] OMAPDSS: DISPC: set irq_safe for runtime PM
From: Rob Clark @ 2013-09-19 15:14 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1379585958-24537-1-git-send-email-tomi.valkeinen@ti.com>

On Thu, Sep 19, 2013 at 6:19 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> We have a bug with omapdrm, where omapdrm calls dispc's pm_runtime
> function in atomic context, and dispc's pm_runtime is not marked as
> irq_safe:
>
> BUG: sleeping function called from invalid context at drivers/base/power/runtime.c:952
>
> Dispc's runtime PM callbacks are irq safe, so we can just set the
> irq_safe flag to fix the issue.
>
> However, in the long term, I'd rather have omapdrm manage the runtime PM
> calls in a better way. Calling get/put for every small operation that
> touches the dispc registers is very inefficient. It'd be better and
> cleaner to have clear "in-use" and "not-in-use" states for dispc, so
> that we don't need to do register context restore for small operations,
> only to turn dispc off right afterwards.

hmm, quick mid-conference thought:  in msm, in crtc->prepare and
crtc->dpms(ON) I do clk_prepare_enable(), and inverse in
commit()/dpms(OFF).  (The extra ref held between prepare() and
commit() avoids turning things off mid-modeset.)  The net effect is
that we hold things on when the display is on.  Probably we should do
something similar in omapdrm?

BR,
-R


> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/video/omap2/dss/dispc.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 02a7340..4779750 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -3691,6 +3691,7 @@ static int __init omap_dispchw_probe(struct platform_device *pdev)
>         }
>
>         pm_runtime_enable(&pdev->dev);
> +       pm_runtime_irq_safe(&pdev->dev);
>
>         r = dispc_runtime_get();
>         if (r)
> --
> 1.8.1.2
>

^ permalink raw reply

* Re: [PATCH] pwm-backlight: allow for non-increasing brightness levels
From: Mike Dunn @ 2013-09-19 16:13 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Richard Purdie, Jingoo Han, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, Grant Likely, Rob Herring,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Robert Jarzmik, Marek Vasut
In-Reply-To: <20130919115650.GE10852@ulmo>

On 09/19/2013 04:56 AM, Thierry Reding wrote:
> On Thu, Sep 12, 2013 at 11:35:52AM -0700, Mike Dunn wrote:
>> Currently the driver assumes that the values specified in the brightness-levels
>> device tree property increase as they are parsed from left to right.  But boards
>> that invert the signal between the PWM output and the backlight will need to
>> specify decreasing brightness-levels.  This patch removes the assumption that
>> the last element of the array is the max value, and instead searches the array
>> for the max value and uses that as the normalizing value when determining the
>> duty cycle.
> 
> "maximum value", "... and uses that as the scale to normalize the duty
> cycle"?


It's been a while since my last math class... is "normalizing value" not the
correct term?  Maybe just "uses that in the duty cycle calculation"?


> 
> Also please wrap commit messages at 72 characters.


OK.  Sorry, didn't know.


> 
>> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
>> index 1fea627..d66aaa0 100644
>> --- a/drivers/video/backlight/pwm_bl.c
>> +++ b/drivers/video/backlight/pwm_bl.c
>> @@ -27,6 +27,7 @@ struct pwm_bl_data {
>>  	unsigned int		period;
>>  	unsigned int		lth_brightness;
>>  	unsigned int		*levels;
>> +	unsigned int		max_level;
> 
> Perhaps call this "scale"? Otherwise there some potential to mix it up
> with max_brightness.


Yes, this name is thorny.  The code was somewhat confusing to me until I
realized that for the DT case, brightness and max_brightness are indices into
the levels[] array, whereas they are actual values for the platform_data case.
I'll go with "scale" if you prefer.


> 
>> @@ -195,7 +196,15 @@ static int pwm_backlight_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	if (data->levels) {
>> -		max = data->levels[data->max_brightness];
>> +		int i, max_value = 0, max_idx = 0;
> 
> i should be unsigned int to match the type of data->max_brightness.


Yes, thanks.  I'm surprised there's no warning from the compiler.  I'm also
assigning an unsigned to a signed.


> 
>> +		for (i = 0; i <= data->max_brightness; i++) {
> 
> There should be a blank line above this one to increase readability.
> 
>> +			if (data->levels[i] > max_value) {
>> +				max_value = data->levels[i];
>> +				max_idx = i;
>> +			}
>> +		}
>> +		pb->max_level = max_idx;
> 
> Some here.
> 
> Also I suggest to just drop the max_ prefix from the local variables.
> Perhaps also simplify all of it to something like:
> 
> 	for (i = 0; i <= data->max_brightness; i++)
> 		if (data->levels[i] > pb->scale)
> 			pb->scale = data->levels[i];
> 
> And get rid of the index altogether. That way you can use pb->scale
> directly during the computation of the duty cycle and don't have to
> index the levels array over and over again.


Ok, if you prefer.  The reason I made max_level an index is for consistency.
For the DT case, brightness and max_brightness are indices, and I had already
been confused by the value-versus-index issue.

Thanks much for the review!  I'll ready a v2 patch.

Mike


^ permalink raw reply

* [PATCH 00/51] DMA mask changes
From: Russell King - ARM Linux @ 2013-09-19 21:22 UTC (permalink / raw)
  To: 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

This started out as a request to look at the DMA mask situation, and how
to solve the issues which we have on ARM - notably how the DMA mask
should be setup.

However, I started off reviewing how the dma_mask and coherent_dma_mask
was being used, and what I found was rather messy, and in some cases
rather buggy.  I tried to get some of the bug fixes in before the last
merge window, but it seems that the maintainers preferred to have the
full solution rather than a simple -rc suitable bug fix.

So, this is an attempt to clean things up.

The first point here is that drivers performing DMA should be calling
dma_set_mask()/dma_set_coherent_mask() in their probe function to verify
that DMA can be performed.  Lots of ARM drivers omit this step; please
refer to the DMA API documentation on this subject.

What this means is that the DMA mask provided by bus code is a default
value - nothing more.  It doesn't have to accurately reflect what the
device is actually capable of.  Apart from the storage for dev->dma_mask
being initialised for any device which is DMA capable, there is no other
initialisation which is strictly necessary at device creation time.

Now, these cleanups address two major areas:
1. The setting of DMA masks, particularly when both the coherent and
   streaming DMA masks are set together.

2. The initialisation of DMA masks by drivers - this seems to be becoming
   a popular habbit, one which may not be entirely the right solution.
   Rather than having this scattered throughout the tree, I've pulled
   that into a central location (and called it coercing the DMA mask -
   because it really is about forcing the DMA mask to be that value.)

3. Finally, addressing the long held misbelief that DMA masks somehow
   correspond with physical addresses.  We already have established
   long ago that dma_addr_t values returned from the DMA API are the
   values which you program into the DMA controller, and so are the
   bus addresses.  It is _only_ sane that DMA masks are also bus
   related too, and not related to physical address spaces.

(3) is a very important point for LPAE systems, which may still have
less than 4GB of memory, but this memory is all located above the 4GB
physical boundary.  This means with the current model, any device
using a 32-bit DMA mask fails - even though the DMA controller is
still only a 32-bit DMA controller but the 32-bit bus addresses map
to system memory.  To put it another way, the bus addresses have a
4GB physical offset on them.

This email is only being sent to the mailing lists in question, not to
anyone personally.  The list of individuals is far to great to do that.
I'm hoping no mailing lists reject the patches based on the number of
recipients.

Patches based on v3.12-rc1.

 Documentation/DMA-API-HOWTO.txt                   |   37 +++++++++------
 Documentation/DMA-API.txt                         |    8 +++
 arch/arm/include/asm/dma-mapping.h                |    8 +++
 arch/arm/mm/dma-mapping.c                         |   49 ++++++++++++++++++--
 arch/arm/mm/init.c                                |   12 +++---
 arch/arm/mm/mm.h                                  |    2 +
 arch/powerpc/kernel/vio.c                         |    3 +-
 block/blk-settings.c                              |    8 ++--
 drivers/amba/bus.c                                |    6 +--
 drivers/ata/pata_ixp4xx_cf.c                      |    5 ++-
 drivers/ata/pata_octeon_cf.c                      |    5 +-
 drivers/block/nvme-core.c                         |   10 ++---
 drivers/crypto/ixp4xx_crypto.c                    |   48 ++++++++++----------
 drivers/dma/amba-pl08x.c                          |    5 ++
 drivers/dma/dw/platform.c                         |    8 +--
 drivers/dma/edma.c                                |    6 +--
 drivers/dma/pl330.c                               |    4 ++
 drivers/firmware/dcdbas.c                         |   23 +++++-----
 drivers/firmware/google/gsmi.c                    |   13 +++--
 drivers/gpu/drm/exynos/exynos_drm_drv.c           |    6 ++-
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c          |    5 +-
 drivers/media/platform/omap3isp/isp.c             |    6 +-
 drivers/media/platform/omap3isp/isp.h             |    3 -
 drivers/mmc/card/queue.c                          |    3 +-
 drivers/mmc/host/sdhci-acpi.c                     |    5 +-
 drivers/net/ethernet/broadcom/b44.c               |    3 +-
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c  |    8 +---
 drivers/net/ethernet/brocade/bna/bnad.c           |   13 ++----
 drivers/net/ethernet/emulex/benet/be_main.c       |   12 +----
 drivers/net/ethernet/intel/e1000/e1000_main.c     |    9 +---
 drivers/net/ethernet/intel/e1000e/netdev.c        |   18 +++-----
 drivers/net/ethernet/intel/igb/igb_main.c         |   18 +++-----
 drivers/net/ethernet/intel/igbvf/netdev.c         |   18 +++-----
 drivers/net/ethernet/intel/ixgb/ixgb_main.c       |   16 ++-----
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |   15 ++----
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   15 ++----
 drivers/net/ethernet/nxp/lpc_eth.c                |    6 ++-
 drivers/net/ethernet/octeon/octeon_mgmt.c         |    5 +-
 drivers/net/ethernet/sfc/efx.c                    |   12 +-----
 drivers/net/wireless/b43/dma.c                    |    9 +---
 drivers/net/wireless/b43legacy/dma.c              |    9 +---
 drivers/of/platform.c                             |    3 -
 drivers/parport/parport_pc.c                      |    8 +++-
 drivers/scsi/scsi_lib.c                           |    2 +-
 drivers/staging/dwc2/platform.c                   |    5 +-
 drivers/staging/et131x/et131x.c                   |   17 +------
 drivers/staging/imx-drm/imx-drm-core.c            |    8 +++-
 drivers/staging/imx-drm/ipuv3-crtc.c              |    4 +-
 drivers/staging/media/dt3155v4l/dt3155v4l.c       |    5 +--
 drivers/usb/chipidea/ci_hdrc_imx.c                |    7 +--
 drivers/usb/dwc3/dwc3-exynos.c                    |    7 +--
 drivers/usb/gadget/lpc32xx_udc.c                  |    4 +-
 drivers/usb/host/bcma-hcd.c                       |    3 +-
 drivers/usb/host/ehci-atmel.c                     |    7 +--
 drivers/usb/host/ehci-octeon.c                    |    4 +-
 drivers/usb/host/ehci-omap.c                      |   10 ++--
 drivers/usb/host/ehci-orion.c                     |    7 +--
 drivers/usb/host/ehci-platform.c                  |   10 ++--
 drivers/usb/host/ehci-s5p.c                       |    7 +--
 drivers/usb/host/ehci-spear.c                     |    7 +--
 drivers/usb/host/ehci-tegra.c                     |    7 +--
 drivers/usb/host/ohci-at91.c                      |    9 ++--
 drivers/usb/host/ohci-exynos.c                    |    7 +--
 drivers/usb/host/ohci-nxp.c                       |    5 +-
 drivers/usb/host/ohci-octeon.c                    |    5 +-
 drivers/usb/host/ohci-omap3.c                     |   10 ++--
 drivers/usb/host/ohci-pxa27x.c                    |    8 ++--
 drivers/usb/host/ohci-sa1111.c                    |    6 +++
 drivers/usb/host/ohci-spear.c                     |    7 +--
 drivers/usb/host/ssb-hcd.c                        |    3 +-
 drivers/usb/host/uhci-platform.c                  |    7 +--
 drivers/usb/musb/am35x.c                          |   50 +++++++--------------
 drivers/usb/musb/da8xx.c                          |   49 +++++++-------------
 drivers/usb/musb/davinci.c                        |   48 +++++++-------------
 drivers/usb/musb/tusb6010.c                       |   49 +++++++-------------
 drivers/video/amba-clcd.c                         |    5 ++
 include/linux/amba/bus.h                          |    2 -
 include/linux/dma-mapping.h                       |   31 +++++++++++++
 sound/arm/pxa2xx-pcm.c                            |    9 +---
 sound/soc/atmel/atmel-pcm.c                       |   11 ++---
 sound/soc/blackfin/bf5xx-ac97-pcm.c               |   11 ++---
 sound/soc/blackfin/bf5xx-i2s-pcm.c                |   10 ++---
 sound/soc/davinci/davinci-pcm.c                   |    9 +---
 sound/soc/fsl/fsl_dma.c                           |    9 +---
 sound/soc/fsl/mpc5200_dma.c                       |   10 ++---
 sound/soc/jz4740/jz4740-pcm.c                     |   12 ++---
 sound/soc/kirkwood/kirkwood-dma.c                 |    9 +---
 sound/soc/nuc900/nuc900-pcm.c                     |    9 ++--
 sound/soc/omap/omap-pcm.c                         |   11 ++---
 sound/soc/pxa/pxa2xx-pcm.c                        |   11 ++---
 sound/soc/s6000/s6000-pcm.c                       |    9 +---
 sound/soc/samsung/dma.c                           |   11 ++---
 sound/soc/samsung/idma.c                          |   11 ++---
 93 files changed, 493 insertions(+), 566 deletions(-)


^ permalink raw reply

* [PATCH 01/51] DMA-API: provide a helper to set both DMA and coherent DMA masks
From: Russell King @ 2013-09-19 21:25 UTC (permalink / raw)
  To: 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: Vinod Koul, Dan Williams, Rob Landley
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

Provide a helper to set both the DMA and coherent DMA masks to the
same value - this avoids duplicated code in a number of drivers,
sometimes with buggy error handling, and also allows us identify
which drivers do things differently.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 Documentation/DMA-API-HOWTO.txt |   37 ++++++++++++++++++++++---------------
 Documentation/DMA-API.txt       |    8 ++++++++
 include/linux/dma-mapping.h     |   14 ++++++++++++++
 3 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/Documentation/DMA-API-HOWTO.txt b/Documentation/DMA-API-HOWTO.txt
index 14129f1..5e98303 100644
--- a/Documentation/DMA-API-HOWTO.txt
+++ b/Documentation/DMA-API-HOWTO.txt
@@ -101,14 +101,23 @@ style to do this even if your device holds the default setting,
 because this shows that you did think about these issues wrt. your
 device.
 
-The query is performed via a call to dma_set_mask():
+The query is performed via a call to dma_set_mask_and_coherent():
 
-	int dma_set_mask(struct device *dev, u64 mask);
+	int dma_set_mask_and_coherent(struct device *dev, u64 mask);
 
-The query for consistent allocations is performed via a call to
-dma_set_coherent_mask():
+which will query the mask for both streaming and coherent APIs together.
+If you have some special requirements, then the following two separate
+queries can be used instead:
 
-	int dma_set_coherent_mask(struct device *dev, u64 mask);
+	The query for streaming mappings is performed via a call to
+	dma_set_mask():
+
+		int dma_set_mask(struct device *dev, u64 mask);
+
+	The query for consistent allocations is performed via a call
+	to dma_set_coherent_mask():
+
+		int dma_set_coherent_mask(struct device *dev, u64 mask);
 
 Here, dev is a pointer to the device struct of your device, and mask
 is a bit mask describing which bits of an address your device
@@ -137,7 +146,7 @@ exactly why.
 
 The standard 32-bit addressing device would do something like this:
 
-	if (dma_set_mask(dev, DMA_BIT_MASK(32))) {
+	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))) {
 		printk(KERN_WARNING
 		       "mydev: No suitable DMA available.\n");
 		goto ignore_this_device;
@@ -171,22 +180,20 @@ If a card is capable of using 64-bit consistent allocations as well,
 
 	int using_dac, consistent_using_dac;
 
-	if (!dma_set_mask(dev, DMA_BIT_MASK(64))) {
+	if (!dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64))) {
 		using_dac = 1;
 	   	consistent_using_dac = 1;
-		dma_set_coherent_mask(dev, DMA_BIT_MASK(64));
-	} else if (!dma_set_mask(dev, DMA_BIT_MASK(32))) {
+	} else if (!dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))) {
 		using_dac = 0;
 		consistent_using_dac = 0;
-		dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
 	} else {
 		printk(KERN_WARNING
 		       "mydev: No suitable DMA available.\n");
 		goto ignore_this_device;
 	}
 
-dma_set_coherent_mask() will always be able to set the same or a
-smaller mask as dma_set_mask(). However for the rare case that a
+The coherent coherent mask will always be able to set the same or a
+smaller mask as the streaming mask. However for the rare case that a
 device driver only uses consistent allocations, one would have to
 check the return value from dma_set_coherent_mask().
 
@@ -199,9 +206,9 @@ Finally, if your device can only drive the low 24-bits of
 		goto ignore_this_device;
 	}
 
-When dma_set_mask() is successful, and returns zero, the kernel saves
-away this mask you have provided.  The kernel will use this
-information later when you make DMA mappings.
+When dma_set_mask() or dma_set_mask_and_coherent() is successful, and
+returns zero, the kernel saves away this mask you have provided.  The
+kernel will use this information later when you make DMA mappings.
 
 There is a case which we are aware of at this time, which is worth
 mentioning in this documentation.  If your device supports multiple
diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index 78a6c56..e865279 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -142,6 +142,14 @@ internal API for use by the platform than an external API for use by
 driver writers.
 
 int
+dma_set_mask_and_coherent(struct device *dev, u64 mask)
+
+Checks to see if the mask is possible and updates the device
+streaming and coherent DMA mask parameters if it is.
+
+Returns: 0 if successful and a negative error if not.
+
+int
 dma_set_mask(struct device *dev, u64 mask)
 
 Checks to see if the mask is possible and updates the device
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 3a8d0a2..ec951f9 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -97,6 +97,20 @@ static inline int dma_set_coherent_mask(struct device *dev, u64 mask)
 }
 #endif
 
+/*
+ * Set both the DMA mask and the coherent DMA mask to the same thing.
+ * Note that we don't check the return value from dma_set_coherent_mask()
+ * as the DMA API guarantees that the coherent DMA mask can be set to
+ * the same or smaller than the streaming DMA mask.
+ */
+static inline int dma_set_mask_and_coherent(struct device *dev, u64 mask)
+{
+	int rc = dma_set_mask(dev, mask);
+	if (rc = 0)
+		dma_set_coherent_mask(dev, mask);
+	return rc;
+}
+
 extern u64 dma_get_required_mask(struct device *dev);
 
 static inline unsigned int dma_get_max_seg_size(struct device *dev)
-- 
1.7.4.4


^ permalink raw reply related

* [PATCH 02/51] DMA-API: net: brocade/bna/bnad.c: fix 32-bit DMA mask handling
From: Russell King @ 2013-09-19 21:26 UTC (permalink / raw)
  To: 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: Rasesh Mody
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

The fallback to 32-bit DMA mask is rather odd:
	if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) &&
	    !dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64))) {
		*using_dac = true;
	} else {
		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
		if (err) {
			err = dma_set_coherent_mask(&pdev->dev,
						    DMA_BIT_MASK(32));
			if (err)
				goto release_regions;
		}

This means we only try and set the coherent DMA mask if we failed to
set a 32-bit DMA mask, and only if both fail do we fail the driver.
Adjust this so that if either setting fails, we fail the driver - and
thereby end up properly setting both the DMA mask and the coherent
DMA mask in the fallback case.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/brocade/bna/bnad.c |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/brocade/bna/bnad.c b/drivers/net/ethernet/brocade/bna/bnad.c
index b78e69e..45ce6e2 100644
--- a/drivers/net/ethernet/brocade/bna/bnad.c
+++ b/drivers/net/ethernet/brocade/bna/bnad.c
@@ -3300,17 +3300,12 @@ bnad_pci_init(struct bnad *bnad,
 	err = pci_request_regions(pdev, BNAD_NAME);
 	if (err)
 		goto disable_device;
-	if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) &&
-	    !dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64))) {
+	if (!dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) {
 		*using_dac = true;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
-		if (err) {
-			err = dma_set_coherent_mask(&pdev->dev,
-						    DMA_BIT_MASK(32));
-			if (err)
-				goto release_regions;
-		}
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+		if (err)
+			goto release_regions;
 		*using_dac = false;
 	}
 	pci_set_master(pdev);
-- 
1.7.4.4


^ permalink raw reply related

* [PATCH 03/51] DMA-API: net: intel/e1000e: fix 32-bit DMA mask handling
From: Russell King @ 2013-09-19 21:27 UTC (permalink / raw)
  To: 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: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Peter P Waskiewicz Jr, Alex Duyck,
	John Ronciak, Tushar Dave
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

The fallback to 32-bit DMA mask is rather odd:
	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
	if (!err) {
		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
		if (!err)
			pci_using_dac = 1;
	} else {
		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
		if (err) {
			err = dma_set_coherent_mask(&pdev->dev,
						    DMA_BIT_MASK(32));
			if (err) {
				dev_err(&pdev->dev,
					"No usable DMA configuration, aborting\n");
				goto err_dma;
			}
		}
	}
This means we only set the coherent DMA mask in the fallback path if
the DMA mask set failed, which is silly.  This fixes it to set the
coherent DMA mask only if dma_set_mask() succeeded, and to error out
if either fails.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/intel/e1000e/netdev.c |   18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index e87e9b0..519e293 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6553,21 +6553,15 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return err;
 
 	pci_using_dac = 0;
-	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
+	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (!err) {
-		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
-		if (!err)
-			pci_using_dac = 1;
+		pci_using_dac = 1;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
-			err = dma_set_coherent_mask(&pdev->dev,
-						    DMA_BIT_MASK(32));
-			if (err) {
-				dev_err(&pdev->dev,
-					"No usable DMA configuration, aborting\n");
-				goto err_dma;
-			}
+			dev_err(&pdev->dev,
+				"No usable DMA configuration, aborting\n");
+			goto err_dma;
 		}
 	}
 
-- 
1.7.4.4


^ permalink raw reply related

* [PATCH 04/51] DMA-API: net: intel/igb: fix 32-bit DMA mask handling
From: Russell King @ 2013-09-19 21:28 UTC (permalink / raw)
  To: 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: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Peter P Waskiewicz Jr, Alex Duyck,
	John Ronciak, Tushar Dave
In-Reply-To: <20130919212235.GD12758-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

The fallback to 32-bit DMA mask is rather odd:
	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
	if (!err) {
		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
		if (!err)
			pci_using_dac = 1;
	} else {
		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
		if (err) {
			err = dma_set_coherent_mask(&pdev->dev,
						    DMA_BIT_MASK(32));
			if (err) {
				dev_err(&pdev->dev,
					"No usable DMA configuration, aborting\n");
				goto err_dma;
			}
		}
	}
This means we only set the coherent DMA mask in the fallback path if
the DMA mask set failed, which is silly.  This fixes it to set the
coherent DMA mask only if dma_set_mask() succeeded, and to error out
if either fails.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/intel/igb/igb_main.c |   18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 8cf44f2..7579383 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2034,21 +2034,15 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return err;
 
 	pci_using_dac = 0;
-	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
+	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (!err) {
-		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
-		if (!err)
-			pci_using_dac = 1;
+		pci_using_dac = 1;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
-			err = dma_set_coherent_mask(&pdev->dev,
-						    DMA_BIT_MASK(32));
-			if (err) {
-				dev_err(&pdev->dev,
-					"No usable DMA configuration, aborting\n");
-				goto err_dma;
-			}
+			dev_err(&pdev->dev,
+				"No usable DMA configuration, aborting\n");
+			goto err_dma;
 		}
 	}
 
-- 
1.7.4.4


^ permalink raw reply related

* [PATCH 05/51] DMA-API: net: intel/igbvf: fix 32-bit DMA mask handling
From: Russell King @ 2013-09-19 21:29 UTC (permalink / raw)
  To: 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: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Peter P Waskiewicz Jr, Alex Duyck,
	John Ronciak, Tushar Dave
In-Reply-To: <20130919212235.GD12758-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

The fallback to 32-bit DMA mask is rather odd:
	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
	if (!err) {
		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
		if (!err)
			pci_using_dac = 1;
	} else {
		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
		if (err) {
			err = dma_set_coherent_mask(&pdev->dev,
						    DMA_BIT_MASK(32));
			if (err) {
				dev_err(&pdev->dev, "No usable DMA "
					"configuration, aborting\n");
				goto err_dma;
			}
		}
	}
This means we only set the coherent DMA mask in the fallback path if
the DMA mask set failed, which is silly.  This fixes it to set the
coherent DMA mask only if dma_set_mask() succeeded, and to error out
if either fails.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/intel/igbvf/netdev.c |   18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
index 93eb7ee..4e6b02f 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -2638,21 +2638,15 @@ static int igbvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return err;
 
 	pci_using_dac = 0;
-	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
+	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (!err) {
-		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
-		if (!err)
-			pci_using_dac = 1;
+		pci_using_dac = 1;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
-			err = dma_set_coherent_mask(&pdev->dev,
-						    DMA_BIT_MASK(32));
-			if (err) {
-				dev_err(&pdev->dev, "No usable DMA "
-				        "configuration, aborting\n");
-				goto err_dma;
-			}
+			dev_err(&pdev->dev, "No usable DMA "
+			        "configuration, aborting\n");
+			goto err_dma;
 		}
 	}
 
-- 
1.7.4.4


^ permalink raw reply related

* [PATCH 06/51] DMA-API: net: intel/ixgb: fix 32-bit DMA mask handling
From: Russell King @ 2013-09-19 21:30 UTC (permalink / raw)
  To: 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: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Peter P Waskiewicz Jr, Alex Duyck,
	John Ronciak, Tushar Dave
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

The fallback to 32-bit DMA mask is rather odd:
	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
	if (!err) {
		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
		if (!err)
			pci_using_dac = 1;
	} else {
		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
		if (err) {
			err = dma_set_coherent_mask(&pdev->dev,
						    DMA_BIT_MASK(32));
			if (err) {
				pr_err("No usable DMA configuration, aborting\n");
				goto err_dma_mask;
			}
		}
	}
This means we only set the coherent DMA mask in the fallback path if
the DMA mask set failed, which is silly.  This fixes it to set the
coherent DMA mask only if dma_set_mask() succeeded, and to error out
if either fails.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/intel/ixgb/ixgb_main.c |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
index 9f6b236..57e390c 100644
--- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
+++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
@@ -408,20 +408,14 @@ ixgb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return err;
 
 	pci_using_dac = 0;
-	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
+	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (!err) {
-		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
-		if (!err)
-			pci_using_dac = 1;
+		pci_using_dac = 1;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
-			err = dma_set_coherent_mask(&pdev->dev,
-						    DMA_BIT_MASK(32));
-			if (err) {
-				pr_err("No usable DMA configuration, aborting\n");
-				goto err_dma_mask;
-			}
+			pr_err("No usable DMA configuration, aborting\n");
+			goto err_dma_mask;
 		}
 	}
 
-- 
1.7.4.4


^ permalink raw reply related

* [PATCH 07/51] DMA-API: net: intel/ixgbe: fix 32-bit DMA mask handling
From: Russell King @ 2013-09-19 21:31 UTC (permalink / raw)
  To: 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: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Peter P Waskiewicz Jr, Alex Duyck,
	John Ronciak, Tushar Dave
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

The fallback to 32-bit DMA mask is rather odd:
	if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) &&
	    !dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64))) {
		pci_using_dac = 1;
	} else {
		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
		if (err) {
			err = dma_set_coherent_mask(&pdev->dev,
						    DMA_BIT_MASK(32));
			if (err) {
				dev_err(&pdev->dev,
					"No usable DMA configuration, aborting\n");
				goto err_dma;
			}
		}
		pci_using_dac = 0;
	}
This means we only set the coherent DMA mask in the fallback path if
the DMA mask set failed, which is silly.  This fixes it to set the
coherent DMA mask only if dma_set_mask() succeeded, and to error out
if either fails.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 7aba452..b1dc844 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7475,19 +7475,14 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err)
 		return err;
 
-	if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) &&
-	    !dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64))) {
+	if (!dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) {
 		pci_using_dac = 1;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
-			err = dma_set_coherent_mask(&pdev->dev,
-						    DMA_BIT_MASK(32));
-			if (err) {
-				dev_err(&pdev->dev,
-					"No usable DMA configuration, aborting\n");
-				goto err_dma;
-			}
+			dev_err(&pdev->dev,
+				"No usable DMA configuration, aborting\n");
+			goto err_dma;
 		}
 		pci_using_dac = 0;
 	}
-- 
1.7.4.4


^ permalink raw reply related

* [PATCH 08/51] DMA-API: net: intel/ixgbevf: fix 32-bit DMA mask handling
From: Russell King @ 2013-09-19 21:32 UTC (permalink / raw)
  To: 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: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Peter P Waskiewicz Jr, Alex Duyck,
	John Ronciak, Tushar Dave
In-Reply-To: <20130919212235.GD12758-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

The fallback to 32-bit DMA mask is rather odd:
	if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) &&
	    !dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64))) {
		pci_using_dac = 1;
	} else {
		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
		if (err) {
			err = dma_set_coherent_mask(&pdev->dev,
						    DMA_BIT_MASK(32));
			if (err) {
				dev_err(&pdev->dev, "No usable DMA "
					"configuration, aborting\n");
				goto err_dma;
			}
		}
		pci_using_dac = 0;
	}
This means we only set the coherent DMA mask in the fallback path if
the DMA mask set failed, which is silly.  This fixes it to set the
coherent DMA mask only if dma_set_mask() succeeded, and to error out
if either fails.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 59a62bb..e34c2da 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -3326,19 +3326,14 @@ static int ixgbevf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err)
 		return err;
 
-	if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) &&
-	    !dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64))) {
+	if (!dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) {
 		pci_using_dac = 1;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
-			err = dma_set_coherent_mask(&pdev->dev,
-						    DMA_BIT_MASK(32));
-			if (err) {
-				dev_err(&pdev->dev, "No usable DMA "
-					"configuration, aborting\n");
-				goto err_dma;
-			}
+			dev_err(&pdev->dev, "No usable DMA "
+				"configuration, aborting\n");
+			goto err_dma;
 		}
 		pci_using_dac = 0;
 	}
-- 
1.7.4.4


^ permalink raw reply related

* [PATCH 09/51] DMA-API: net: broadcom/b44: replace dma_set_mask()+dma_set_coherent_mask() with new
From: Russell King @ 2013-09-19 21:33 UTC (permalink / raw)
  To: 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: Gary Zambrano
In-Reply-To: <20130919212235.GD12758-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

Replace the following sequence:

	dma_set_mask(dev, mask);
	dma_set_coherent_mask(dev, mask);

with a call to the new helper dma_set_mask_and_coherent().

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/broadcom/b44.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index 9b017d9..b4d2018 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -2183,8 +2183,7 @@ static int b44_init_one(struct ssb_device *sdev,
 		goto err_out_free_dev;
 	}
 
-	if (dma_set_mask(sdev->dma_dev, DMA_BIT_MASK(30)) ||
-	    dma_set_coherent_mask(sdev->dma_dev, DMA_BIT_MASK(30))) {
+	if (dma_set_mask_and_coherent(sdev->dma_dev, DMA_BIT_MASK(30))) {
 		dev_err(sdev->dev,
 			"Required 30BIT DMA mask unsupported by the system\n");
 		goto err_out_powerdown;
-- 
1.7.4.4


^ permalink raw reply related

* [PATCH 10/51] DMA-API: net: broadcom/bnx2x: replace dma_set_mask()+dma_set_coherent_mask() with new
From: Russell King @ 2013-09-19 21:34 UTC (permalink / raw)
  To: 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: Eilon Greenstein
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

Replace the following sequence:

	dma_set_mask(dev, mask);
	dma_set_coherent_mask(dev, mask);

with a call to the new helper dma_set_mask_and_coherent().

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 2f8dbbb..e6c3e66 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -12072,13 +12072,9 @@ static int bnx2x_set_coherency_mask(struct bnx2x *bp)
 {
 	struct device *dev = &bp->pdev->dev;
 
-	if (dma_set_mask(dev, DMA_BIT_MASK(64)) = 0) {
+	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)) = 0) {
 		bp->flags |= USING_DAC_FLAG;
-		if (dma_set_coherent_mask(dev, DMA_BIT_MASK(64)) != 0) {
-			dev_err(dev, "dma_set_coherent_mask failed, aborting\n");
-			return -EIO;
-		}
-	} else if (dma_set_mask(dev, DMA_BIT_MASK(32)) != 0) {
+	} else if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)) != 0) {
 		dev_err(dev, "System does not support DMA, aborting\n");
 		return -EIO;
 	}
-- 
1.7.4.4


^ permalink raw reply related


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