* Re: [PATCH] fbdev: radeon: Remove 'struct timeval' usage
From: Dave Airlie @ 2015-06-05 4:40 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Tina Ruchandani, Arnd Bergmann, y2038, Benjamin Herrenschmidt,
Jean-Christophe Plagniol-Villard, Linux Fbdev development list,
LKML
In-Reply-To: <556EEC3D.4010608@ti.com>
On 3 June 2015 at 21:59, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
>
> On 25/05/15 07:07, Tina Ruchandani wrote:
>> 'struct timeval' uses a 32-bit representation for the
>> seconds field which will overflow in the year 2038 and beyond.
>> This patch replaces the usage of 'struct timeval' with
>> ktime_t which uses a 64-bit time representation and does not
>> suffer from the y2038 problem. This patch is part of a larger
>> effort to remove all instances of 'struct timeval', 'struct
>> timespec', time_t and other 32-bit timekeeping variables
>> from the kernel.
>> The patch also replaces the use of real time (do_gettimeofday)
>> with monotonic time (ktime_get).
>>
>> Signed-off-by: Tina Ruchandani <ruchandani.tina@gmail.com>
>> ---
>> drivers/video/fbdev/aty/radeon_base.c | 29 ++++++++++++++---------------
>> 1 file changed, 14 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
>> index 01237c8..9747e9e 100644
>> --- a/drivers/video/fbdev/aty/radeon_base.c
>> +++ b/drivers/video/fbdev/aty/radeon_base.c
>> @@ -64,6 +64,7 @@
>> #include <linux/slab.h>
>> #include <linux/delay.h>
>> #include <linux/time.h>
>> +#include <linux/ktime.h>
>> #include <linux/fb.h>
>> #include <linux/ioport.h>
>> #include <linux/init.h>
>> @@ -461,8 +462,8 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
>> int hTotal, vTotal, num, denom, m, n;
>> unsigned long long hz, vclk;
>> long xtal;
>> - struct timeval start_tv, stop_tv;
>> - long total_secs, total_usecs;
>> + ktime_t start, stop;
>> + s64 delta;
>> int i;
>>
>> /* Ugh, we cut interrupts, bad bad bad, but we want some precision
>> @@ -478,7 +479,7 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
>> if (((INREG(CRTC_VLINE_CRNT_VLINE) >> 16) & 0x3ff) = 0)
>> break;
>>
>> - do_gettimeofday(&start_tv);
>> + start = ktime_get();
>>
>> for(i=0; i<1000000; i++)
>> if (((INREG(CRTC_VLINE_CRNT_VLINE) >> 16) & 0x3ff) != 0)
>> @@ -487,20 +488,18 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
>> for(i=0; i<1000000; i++)
>> if (((INREG(CRTC_VLINE_CRNT_VLINE) >> 16) & 0x3ff) = 0)
>> break;
>> -
>> - do_gettimeofday(&stop_tv);
>> -
>> +
>> + stop = ktime_get();
>> +
>> local_irq_enable();
>>
>> - total_secs = stop_tv.tv_sec - start_tv.tv_sec;
>> - if (total_secs > 10)
>> + delta = ktime_us_delta(stop, start);
>> +
>> + /* Return -1 if more than 10 seconds have elapsed */
>> + if (delta > (10*1000000))
>> return -1;
>> - total_usecs = stop_tv.tv_usec - start_tv.tv_usec;
>> - total_usecs += total_secs * 1000000;
>> - if (total_usecs < 0)
>> - total_usecs = -total_usecs;
>> - hz = 1000000/total_usecs;
>> -
>> + hz = 1000000/delta;
This needs to be on of the do_div family.
Dave.
^ permalink raw reply
* Re: [PATCH] fbdev: radeon: Remove 'struct timeval' usage
From: Tina Ruchandani @ 2015-06-05 7:55 UTC (permalink / raw)
To: Dave Airlie
Cc: Tomi Valkeinen, Arnd Bergmann, y2038, Benjamin Herrenschmidt,
Jean-Christophe Plagniol-Villard, Linux Fbdev development list,
LKML
In-Reply-To: <20150525040716.GA4448@tinar>
>>> + hz = 1000000/delta;
>
> This needs to be on of the do_div family.
>
> Dave.
Hi Dave,
I build-tested the patch for both 32-bit and 64-bit x86. If my
understanding is correct, since the divisor is 64-bit here, the
compiler will do "if (delta > 1000000) hz = 0; else hz 1000000/(s32)delta" automatically?
In general, is this a good thumb-rule to follow - use do_div if the
dividend is 64-bit, and normal divide operator if only the divisor is
64-bit?
Tina
^ permalink raw reply
* [PATCH -next] fbdev: radeon: Avoid 64 bit divide
From: Guenter Roeck @ 2015-06-05 18:05 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel, Guenter Roeck, Tina Ruchandani
A 64 bit divide causes build failures with 32 bit builds, such as
ERROR: "__divdi3" [drivers/video/fbdev/aty/radeonfb.ko] undefined!
or
drivers/built-in.o: In function `radeon_probe_pll_params':
radeon_base.c:(.text+0x16608b): undefined reference to `__divdi3'
Fixes: ce8f4069dce8 ("fbdev: radeon: Remove 'struct timeval' usage")
Cc: Tina Ruchandani <ruchandani.tina@gmail.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/video/fbdev/aty/radeon_base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
index b3612e30d74f..374e47532035 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -492,7 +492,7 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
/* Return -1 if more than 10 seconds have elapsed */
if (delta > (10*1000000))
return -1;
- hz = 1000000/delta;
+ hz = 1000000 / (int)delta;
hTotal = ((INREG(CRTC_H_TOTAL_DISP) & 0x1ff) + 1) * 8;
vTotal = ((INREG(CRTC_V_TOTAL_DISP) & 0x3ff) + 1);
--
2.1.0
^ permalink raw reply related
* Re: [PATCH -next] fbdev: radeon: Avoid 64 bit divide
From: Benjamin Herrenschmidt @ 2015-06-05 20:08 UTC (permalink / raw)
To: Guenter Roeck
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel, Tina Ruchandani
In-Reply-To: <1433527521-5384-1-git-send-email-linux@roeck-us.net>
On Fri, 2015-06-05 at 11:05 -0700, Guenter Roeck wrote:
> A 64 bit divide causes build failures with 32 bit builds, such as
>
> ERROR: "__divdi3" [drivers/video/fbdev/aty/radeonfb.ko] undefined!
>
> or
>
> drivers/built-in.o: In function `radeon_probe_pll_params':
> radeon_base.c:(.text+0x16608b): undefined reference to `__divdi3'
What about using do_div() instead ?
> Fixes: ce8f4069dce8 ("fbdev: radeon: Remove 'struct timeval' usage")
> Cc: Tina Ruchandani <ruchandani.tina@gmail.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/video/fbdev/aty/radeon_base.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
> index b3612e30d74f..374e47532035 100644
> --- a/drivers/video/fbdev/aty/radeon_base.c
> +++ b/drivers/video/fbdev/aty/radeon_base.c
> @@ -492,7 +492,7 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
> /* Return -1 if more than 10 seconds have elapsed */
> if (delta > (10*1000000))
> return -1;
> - hz = 1000000/delta;
> + hz = 1000000 / (int)delta;
>
> hTotal = ((INREG(CRTC_H_TOTAL_DISP) & 0x1ff) + 1) * 8;
> vTotal = ((INREG(CRTC_V_TOTAL_DISP) & 0x3ff) + 1);
^ permalink raw reply
* Re: [PATCH -next] fbdev: radeon: Avoid 64 bit divide
From: Guenter Roeck @ 2015-06-05 20:23 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel, Tina Ruchandani
In-Reply-To: <1433534917.4526.105.camel@kernel.crashing.org>
On 06/05/2015 01:08 PM, Benjamin Herrenschmidt wrote:
> On Fri, 2015-06-05 at 11:05 -0700, Guenter Roeck wrote:
>> A 64 bit divide causes build failures with 32 bit builds, such as
>>
>> ERROR: "__divdi3" [drivers/video/fbdev/aty/radeonfb.ko] undefined!
>>
>> or
>>
>> drivers/built-in.o: In function `radeon_probe_pll_params':
>> radeon_base.c:(.text+0x16608b): undefined reference to `__divdi3'
>
> What about using do_div() instead ?
>
delta is known to be <= 10*1000000, so I figured that would have been overkill.
But, sure, at the end it is the same. Want me to resubmit ?
Guenter
>> Fixes: ce8f4069dce8 ("fbdev: radeon: Remove 'struct timeval' usage")
>> Cc: Tina Ruchandani <ruchandani.tina@gmail.com>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> drivers/video/fbdev/aty/radeon_base.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
>> index b3612e30d74f..374e47532035 100644
>> --- a/drivers/video/fbdev/aty/radeon_base.c
>> +++ b/drivers/video/fbdev/aty/radeon_base.c
>> @@ -492,7 +492,7 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
>> /* Return -1 if more than 10 seconds have elapsed */
>> if (delta > (10*1000000))
>> return -1;
>> - hz = 1000000/delta;
>> + hz = 1000000 / (int)delta;
>>
>> hTotal = ((INREG(CRTC_H_TOTAL_DISP) & 0x1ff) + 1) * 8;
>> vTotal = ((INREG(CRTC_V_TOTAL_DISP) & 0x3ff) + 1);
>
>
>
^ permalink raw reply
* Re: [Y2038] [PATCH] fbdev: radeon: Remove 'struct timeval' usage
From: Arnd Bergmann @ 2015-06-05 21:12 UTC (permalink / raw)
To: y2038
Cc: Tina Ruchandani, Dave Airlie, Linux Fbdev development list,
Benjamin Herrenschmidt, LKML, Tomi Valkeinen,
Jean-Christophe Plagniol-Villard
In-Reply-To: <CAB__kknUjU1vdERZQQCxD95idobKN6M-eQZP1jH77tTS3MxhGA@mail.gmail.com>
On Friday 05 June 2015 00:55:05 Tina Ruchandani wrote:
> >>> + hz = 1000000/delta;
> >
> > This needs to be on of the do_div family.
> >
> > Dave.
>
> Hi Dave,
> I build-tested the patch for both 32-bit and 64-bit x86. If my
> understanding is correct, since the divisor is 64-bit here, the
> compiler will do "if (delta > 1000000) hz = 0; else hz > 1000000/(s32)delta" automatically?
> In general, is this a good thumb-rule to follow - use do_div if the
> dividend is 64-bit, and normal divide operator if only the divisor is
> 64-bit?
>
I got a build error on 32-bit arm now. There is already a check
for an overflow of 10 seconds in there (10 million microseconds),
so it is safe to do the easiest fix is to cast that microsecond
value to a u32.
Arnd
^ permalink raw reply
* [PATCH] fbdev: udlfb: remove unneeded initialization in few places
From: Alexey Klimov @ 2015-06-07 23:02 UTC (permalink / raw)
To: linux-fbdev
Small minor cleanup.
This patch removes unneeded initializations of variables
in few places in different functions and one empty line.
Signed-off-by: Alexey Klimov <klimov.linux@gmail.com>
---
drivers/video/fbdev/udlfb.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/video/fbdev/udlfb.c b/drivers/video/fbdev/udlfb.c
index ff2b873..e9c2f7b 100644
--- a/drivers/video/fbdev/udlfb.c
+++ b/drivers/video/fbdev/udlfb.c
@@ -279,7 +279,7 @@ static int dlfb_set_video_mode(struct dlfb_data *dev,
{
char *buf;
char *wrptr;
- int retval = 0;
+ int retval;
int writesize;
struct urb *urb;
@@ -1505,8 +1505,7 @@ static int dlfb_parse_vendor_descriptor(struct dlfb_data *dev,
char *desc;
char *buf;
char *desc_end;
-
- int total_len = 0;
+ int total_len;
buf = kzalloc(MAX_VENDOR_DESCRIPTOR_SIZE, GFP_KERNEL);
if (!buf)
@@ -1582,7 +1581,7 @@ static int dlfb_usb_probe(struct usb_interface *interface,
const struct usb_device_id *id)
{
struct usb_device *usbdev;
- struct dlfb_data *dev = NULL;
+ struct dlfb_data *dev;
int retval = -ENOMEM;
/* usb initialization */
@@ -1665,7 +1664,6 @@ static void dlfb_init_framebuffer_work(struct work_struct *work)
/* allocates framebuffer driver structure, not framebuffer memory */
info = framebuffer_alloc(0, dev->gdev);
if (!info) {
- retval = -ENOMEM;
pr_err("framebuffer_alloc failed\n");
goto error;
}
@@ -1912,7 +1910,7 @@ static int dlfb_alloc_urb_list(struct dlfb_data *dev, int count, size_t size)
static struct urb *dlfb_get_urb(struct dlfb_data *dev)
{
- int ret = 0;
+ int ret;
struct list_head *entry;
struct urb_node *unode;
struct urb *urb = NULL;
--
2.1.4
^ permalink raw reply related
* RE:
From: Practice Trinity (NHS SOUTH SEFTON CCG) @ 2015-06-08 11:09 UTC (permalink / raw)
To: Practice Trinity (NHS SOUTH SEFTON CCG)
In-Reply-To: <132D0DB4B968F242BE373429794F35C22559D38329@NHS-PCLI-MBC011.AD1.NHS.NET>
$1.5 C.A.D for you email ( leonh2800@gmail.com ) for info
********************************************************************************************************************
This message may contain confidential information. If you are not the intended recipient please inform the
sender that you have received the message in error before deleting it.
Please do not disclose, copy or distribute information in this e-mail or take any action in reliance on its contents:
to do so is strictly prohibited and may be unlawful.
Thank you for your co-operation.
NHSmail is the secure email and directory service available for all NHS staff in England and Scotland
NHSmail is approved for exchanging patient data and other sensitive information with NHSmail and GSi recipients
NHSmail provides an email address for your career in the NHS and can be accessed anywhere
********************************************************************************************************************
^ permalink raw reply
* Re: [PATCH 00/21] On-demand device registration
From: Enrico Weigelt, metux IT consult @ 2015-06-08 12:26 UTC (permalink / raw)
To: Alexander Holler, Grygorii.Strashko@linaro.org, Tomeu Vizoso
Cc: Rob Herring, linux-arm-kernel@lists.infradead.org,
Stéphane Marchesin, Thierry Reding, Dmitry Torokhov,
Grant Likely, Rob Herring, Mark Rutland, Dan Williams,
devicetree@vger.kernel.org, dmaengine, dri-devel, linux-clk,
linux-fbdev@vger.kernel.org, linux-gpio@vger.kernel.org,
linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-pm@vger.kernel.org
In-Reply-To: <5570B783.1050808@ahsoftware.de>
Am 04.06.2015 um 22:39 schrieb Alexander Holler:
> As it seems to have been forgotten or overread, I've mentioned in my
> series of patches last year that, with a few changes, it's possible to
> let the algorithm I've used (dfs) to spit out all drivers which can be
> initialized in parallel.
Unfortunately, I've missed that ... could you please resend you patches?
Boot time reduction is one of the topics on my 2do in several weeks.
cu
--
Enrico Weigelt, metux IT consult
+49-151-27565287
MELAG Medizintechnik oHG Sitz Berlin Registergericht AG Charlottenburg HRA 21333 B
Wichtiger Hinweis: Diese Nachricht kann vertrauliche oder nur für einen begrenzten Personenkreis bestimmte Informationen enthalten. Sie ist ausschließlich für denjenigen bestimmt, an den sie gerichtet worden ist. Wenn Sie nicht der Adressat dieser E-Mail sind, dürfen Sie diese nicht kopieren, weiterleiten, weitergeben oder sie ganz oder teilweise in irgendeiner Weise nutzen. Sollten Sie diese E-Mail irrtümlich erhalten haben, so benachrichtigen Sie bitte den Absender, indem Sie auf diese Nachricht antworten. Bitte löschen Sie in diesem Fall diese Nachricht und alle Anhänge, ohne eine Kopie zu behalten.
Important Notice: This message may contain confidential or privileged information. It is intended only for the person it was addressed to. If you are not the intended recipient of this email you may not copy, forward, disclose or otherwise use it or any part of it in any form whatsoever. If you received this email in error please notify the sender by replying and delete this message and any attachments without retaining a copy.
^ permalink raw reply
* Re: [PATCH 00/21] On-demand device registration
From: Alexander Holler @ 2015-06-08 18:14 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult, Grygorii.Strashko@linaro.org,
Tomeu Vizoso
Cc: Rob Herring, linux-arm-kernel@lists.infradead.org,
Stéphane Marchesin, Thierry Reding, Dmitry Torokhov,
Grant Likely, Rob Herring, Mark Rutland, Dan Williams,
devicetree@vger.kernel.org, dmaengine, dri-devel, linux-clk,
linux-fbdev@vger.kernel.org, linux-gpio@vger.kernel.org,
linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-pm@vger.kernel.org
In-Reply-To: <557589D9.1050008@melag.de>
Am 08.06.2015 um 14:26 schrieb Enrico Weigelt, metux IT consult:
> Am 04.06.2015 um 22:39 schrieb Alexander Holler:
>
> > As it seems to have been forgotten or overread, I've mentioned in my
>> series of patches last year that, with a few changes, it's possible to
>> let the algorithm I've used (dfs) to spit out all drivers which can be
>> initialized in parallel.
>
> Unfortunately, I've missed that ... could you please resend you patches?
> Boot time reduction is one of the topics on my 2do in several weeks.
https://lkml.org/lkml/2014/5/12/452
^ permalink raw reply
* Re: [PATCH 00/21] On-demand device registration
From: Alexander Holler @ 2015-06-08 18:18 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult, Grygorii.Strashko@linaro.org,
Tomeu Vizoso
Cc: Rob Herring, linux-arm-kernel@lists.infradead.org,
Stéphane Marchesin, Thierry Reding, Dmitry Torokhov,
Grant Likely, Rob Herring, Mark Rutland, Dan Williams,
devicetree@vger.kernel.org, dmaengine, dri-devel, linux-clk,
linux-fbdev@vger.kernel.org, linux-gpio@vger.kernel.org,
linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-pm@vger.kernel.org
In-Reply-To: <5575DB8D.50000@ahsoftware.de>
Am 08.06.2015 um 20:14 schrieb Alexander Holler:
> Am 08.06.2015 um 14:26 schrieb Enrico Weigelt, metux IT consult:
>> Am 04.06.2015 um 22:39 schrieb Alexander Holler:
>>
>> > As it seems to have been forgotten or overread, I've mentioned in my
>>> series of patches last year that, with a few changes, it's possible to
>>> let the algorithm I've used (dfs) to spit out all drivers which can be
>>> initialized in parallel.
>>
>> Unfortunately, I've missed that ... could you please resend you patches?
>> Boot time reduction is one of the topics on my 2do in several weeks.
>
> https://lkml.org/lkml/2014/5/12/452
>
And don't forget patch 10/9 which fixed a bug in my previous patch
series and which alos was the reason for the large difference in boot
times with and without deps:
https://lkml.org/lkml/2014/5/13/567
^ permalink raw reply
* Re: [PATCH -next] fbdev: radeon: Avoid 64 bit divide
From: Benjamin Herrenschmidt @ 2015-06-08 21:21 UTC (permalink / raw)
To: Guenter Roeck
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel, Tina Ruchandani
In-Reply-To: <5572053A.6060903@roeck-us.net>
On Fri, 2015-06-05 at 13:23 -0700, Guenter Roeck wrote:
> On 06/05/2015 01:08 PM, Benjamin Herrenschmidt wrote:
> > On Fri, 2015-06-05 at 11:05 -0700, Guenter Roeck wrote:
> >> A 64 bit divide causes build failures with 32 bit builds, such as
> >>
> >> ERROR: "__divdi3" [drivers/video/fbdev/aty/radeonfb.ko] undefined!
> >>
> >> or
> >>
> >> drivers/built-in.o: In function `radeon_probe_pll_params':
> >> radeon_base.c:(.text+0x16608b): undefined reference to `__divdi3'
> >
> > What about using do_div() instead ?
> >
>
> delta is known to be <= 10*1000000, so I figured that would have been overkill.
> But, sure, at the end it is the same. Want me to resubmit ?
Don't bother.
> Guenter
>
> >> Fixes: ce8f4069dce8 ("fbdev: radeon: Remove 'struct timeval' usage")
> >> Cc: Tina Ruchandani <ruchandani.tina@gmail.com>
> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> >> ---
> >> drivers/video/fbdev/aty/radeon_base.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
> >> index b3612e30d74f..374e47532035 100644
> >> --- a/drivers/video/fbdev/aty/radeon_base.c
> >> +++ b/drivers/video/fbdev/aty/radeon_base.c
> >> @@ -492,7 +492,7 @@ static int radeon_probe_pll_params(struct radeonfb_info *rinfo)
> >> /* Return -1 if more than 10 seconds have elapsed */
> >> if (delta > (10*1000000))
> >> return -1;
> >> - hz = 1000000/delta;
> >> + hz = 1000000 / (int)delta;
> >>
> >> hTotal = ((INREG(CRTC_H_TOTAL_DISP) & 0x1ff) + 1) * 8;
> >> vTotal = ((INREG(CRTC_V_TOTAL_DISP) & 0x3ff) + 1);
> >
> >
> >
^ permalink raw reply
* [PATCH] fbdev: omap2: remove potential format string leak
From: Kees Cook @ 2015-06-08 22:30 UTC (permalink / raw)
To: linux-kernel
Cc: Tomi Valkeinen, Jean-Christophe Plagniol-Villard, Kees Cook,
linux-omap, linux-fbdev
Since kobject_init_and_add takes a format string, make sure that the
passed in name cannot be accidentally parsed.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/video/fbdev/omap2/dss/display-sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/omap2/dss/display-sysfs.c b/drivers/video/fbdev/omap2/dss/display-sysfs.c
index 12186557a9d4..6ad0991f8259 100644
--- a/drivers/video/fbdev/omap2/dss/display-sysfs.c
+++ b/drivers/video/fbdev/omap2/dss/display-sysfs.c
@@ -324,7 +324,7 @@ int display_init_sysfs(struct platform_device *pdev)
for_each_dss_dev(dssdev) {
r = kobject_init_and_add(&dssdev->kobj, &display_ktype,
- &pdev->dev.kobj, dssdev->alias);
+ &pdev->dev.kobj, "%s", dssdev->alias);
if (r) {
DSSERR("failed to create sysfs files\n");
omap_dss_put_device(dssdev);
--
1.9.1
--
Kees Cook
Chrome OS Security
^ permalink raw reply related
* [PATCH v6 1/3] ivtv: use arch_phys_wc_add() and require PAT disabled
From: Luis R. Rodriguez @ 2015-06-09 0:20 UTC (permalink / raw)
To: mchehab, bp
Cc: tomi.valkeinen, bhelgaas, linux-media, linux-rdma, linux-kernel,
Luis R. Rodriguez, Andy Walls, Doug Ledford, Andy Lutomirski,
Suresh Siddha, Ingo Molnar, Thomas Gleixner, Juergen Gross,
Daniel Vetter, Dave Airlie, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Dave Hansen, Arnd Bergmann,
Michael S. Tsirkin, Stefan Bader, Ville Syrjälä,
Mel Gorman, Vlastimil Babka, Davidlohr Bueso, konrad.wilk,
ville.syrjala, david.vrabel, jbeulich, toshi.kani,
Roger Pau Monné, linux-fbdev, ivtv-devel, xen-devel
In-Reply-To: <1433809222-28261-1-git-send-email-mcgrof@do-not-panic.com>
From: "Luis R. Rodriguez" <mcgrof@suse.com>
We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.
In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.
This is a worthy comprmise given that the hardware is
really rare these days, and perhaps only some lost souls
in some third world country are expected to be using this
feature of the device driver.
Acked-by: Andy Walls <awalls@md.metrocast.net>
Cc: Andy Walls <awalls@md.metrocast.net>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Stefan Bader <stefan.bader@canonical.com>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Borislav Petkov <bp@suse.de>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: david.vrabel@citrix.com
Cc: jbeulich@suse.com
Cc: toshi.kani@hp.com
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: ivtv-devel@ivtvdriver.org
Cc: linux-media@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
drivers/media/pci/ivtv/Kconfig | 3 +++
drivers/media/pci/ivtv/ivtvfb.c | 58 ++++++++++++++++-------------------------
2 files changed, 26 insertions(+), 35 deletions(-)
diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index dd6ee57e..b2a7f88 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
This is used in the Hauppauge PVR-350 card. There is a driver
homepage at <http://www.ivtvdriver.org>.
+ If you have this hardware you will need to boot with PAT disabled
+ on your x86 systems, use the nopat kernel parameter.
+
To compile this driver as a module, choose M here: the
module will be called ivtvfb.
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 9ff1230..7685ae3 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -44,8 +44,8 @@
#include <linux/ivtvfb.h>
#include <linux/slab.h>
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
+#ifdef CONFIG_X86_64
+#include <asm/pat.h>
#endif
#include "ivtv-driver.h"
@@ -155,12 +155,11 @@ struct osd_info {
/* Buffer size */
u32 video_buffer_size;
-#ifdef CONFIG_MTRR
/* video_base rounded down as required by hardware MTRRs */
unsigned long fb_start_aligned_physaddr;
/* video_base rounded up as required by hardware MTRRs */
unsigned long fb_end_aligned_physaddr;
-#endif
+ int wc_cookie;
/* Store the buffer offset */
int set_osd_coords_x;
@@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
static int ivtvfb_init_io(struct ivtv *itv)
{
struct osd_info *oi = itv->osd_info;
+ /* Find the largest power of two that maps the whole buffer */
+ int size_shift = 31;
mutex_lock(&itv->serialize_lock);
if (ivtv_init_on_first_open(itv)) {
@@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi->video_pbase, oi->video_vbase,
oi->video_buffer_size / 1024);
-#ifdef CONFIG_MTRR
- {
- /* Find the largest power of two that maps the whole buffer */
- int size_shift = 31;
-
- while (!(oi->video_buffer_size & (1 << size_shift))) {
- size_shift--;
- }
- size_shift++;
- oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
- oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
- oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
- oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
- if (mtrr_add(oi->fb_start_aligned_physaddr,
- oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr,
- MTRR_TYPE_WRCOMB, 1) < 0) {
- IVTVFB_INFO("disabled mttr\n");
- oi->fb_start_aligned_physaddr = 0;
- oi->fb_end_aligned_physaddr = 0;
- }
- }
-#endif
-
+ while (!(oi->video_buffer_size & (1 << size_shift)))
+ size_shift--;
+ size_shift++;
+ oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
+ oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
+ oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
+ oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
+ oi->wc_cookie = arch_phys_wc_add(oi->fb_start_aligned_physaddr,
+ oi->fb_end_aligned_physaddr -
+ oi->fb_start_aligned_physaddr);
/* Blank the entire osd. */
memset_io(oi->video_vbase, 0, oi->video_buffer_size);
@@ -1172,14 +1160,7 @@ static void ivtvfb_release_buffers (struct ivtv *itv)
/* Release pseudo palette */
kfree(oi->ivtvfb_info.pseudo_palette);
-
-#ifdef CONFIG_MTRR
- if (oi->fb_end_aligned_physaddr) {
- mtrr_del(-1, oi->fb_start_aligned_physaddr,
- oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr);
- }
-#endif
-
+ arch_phys_wc_del(oi->wc_cookie);
kfree(oi);
itv->osd_info = NULL;
}
@@ -1284,6 +1265,13 @@ static int __init ivtvfb_init(void)
int registered = 0;
int err;
+#ifdef CONFIG_X86_64
+ if (WARN(pat_enabled(),
+ "ivtvfb needs PAT disabled, boot with nopat kernel parameter\n")) {
+ return EINVAL;
+ }
+#endif
+
if (ivtvfb_card_id < -1 || ivtvfb_card_id >= IVTV_MAX_CARDS) {
printk(KERN_ERR "ivtvfb: ivtvfb_card_id parameter is out of range (valid range: -1 - %d)\n",
IVTV_MAX_CARDS - 1);
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* [PATCH v6 2/3] IB/ipath: add counting for MTRR
From: Luis R. Rodriguez @ 2015-06-09 0:20 UTC (permalink / raw)
To: mchehab, bp
Cc: tomi.valkeinen, bhelgaas, linux-media, linux-rdma, linux-kernel,
Luis R. Rodriguez, Toshi Kani, Roland Dreier, Sean Hefty,
Hal Rosenstock, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
Juergen Gross, Daniel Vetter, Andy Lutomirski, Dave Airlie,
Antonino Daplas, Jean-Christophe Plagniol-Villard, infinipath,
linux-fbdev
In-Reply-To: <1433809222-28261-1-git-send-email-mcgrof@do-not-panic.com>
From: "Luis R. Rodriguez" <mcgrof@suse.com>
There is no good reason not to, we eventually delete it as well.
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: Roland Dreier <roland@kernel.org>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: infinipath@intel.com
Cc: linux-rdma@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
drivers/infiniband/hw/ipath/ipath_wc_x86_64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
index 4ad0b93..70c1f3a 100644
--- a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
+++ b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
@@ -127,7 +127,7 @@ int ipath_enable_wc(struct ipath_devdata *dd)
"(addr %llx, len=0x%llx)\n",
(unsigned long long) pioaddr,
(unsigned long long) piolen);
- cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 0);
+ cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 1);
if (cookie < 0) {
{
dev_info(&dd->pcidev->dev,
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* [PATCH v6 3/3] IB/ipath: use arch_phys_wc_add() and require PAT disabled
From: Luis R. Rodriguez @ 2015-06-09 0:20 UTC (permalink / raw)
To: mchehab, bp
Cc: tomi.valkeinen, bhelgaas, linux-media, linux-rdma, linux-kernel,
Luis R. Rodriguez, Doug Ledford, Andy Walls, Hal Rosenstock,
Sean Hefty, Suresh Siddha, Rickard Strandqvist, Mike Marciniszyn,
Roland Dreier, Andy Lutomirski, Ingo Molnar, Linus Torvalds,
Thomas Gleixner, Juergen Gross, Daniel Vetter, Dave Airlie,
Antonino Daplas, Jean-Christophe Plagniol-Villard,
Ville Syrjälä, Mel Gorman, Vlastimil Babka,
Davidlohr Bueso, Dave Hansen, Arnd Bergmann, Michael S. Tsirkin,
Stefan Bader, konrad.wilk, ville.syrjala, david.vrabel, jbeulich,
toshi.kani, Roger Pau Monné, infinipath, linux-fbdev,
xen-devel
In-Reply-To: <1433809222-28261-1-git-send-email-mcgrof@do-not-panic.com>
From: "Luis R. Rodriguez" <mcgrof@suse.com>
We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.
In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.
This is a worthy compromise given that the ipath device
driver powers the old HTX bus cards that only work in
AMD systems, while the newer IB/qib device driver
powers all PCI-e cards. The ipath device driver is
obsolete, hardware hard to find and because of this
this its a reasonable compromise to make to require
users of ipath to boot with nopat.
Acked-by: Doug Ledford <dledford@redhat.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Andy Walls <awalls@md.metrocast.net>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Cc: Mike Marciniszyn <mike.marciniszyn@intel.com>
Cc: Roland Dreier <roland@purestorage.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Borislav Petkov <bp@suse.de>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Stefan Bader <stefan.bader@canonical.com>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: david.vrabel@citrix.com
Cc: jbeulich@suse.com
Cc: toshi.kani@hp.com
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: infinipath@intel.com
Cc: linux-rdma@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
drivers/infiniband/hw/ipath/Kconfig | 3 ++
drivers/infiniband/hw/ipath/ipath_driver.c | 18 +++++++----
drivers/infiniband/hw/ipath/ipath_kernel.h | 4 +--
drivers/infiniband/hw/ipath/ipath_wc_x86_64.c | 43 ++++++---------------------
4 files changed, 26 insertions(+), 42 deletions(-)
diff --git a/drivers/infiniband/hw/ipath/Kconfig b/drivers/infiniband/hw/ipath/Kconfig
index 1d9bb11..8fe54ff 100644
--- a/drivers/infiniband/hw/ipath/Kconfig
+++ b/drivers/infiniband/hw/ipath/Kconfig
@@ -9,3 +9,6 @@ config INFINIBAND_IPATH
as IP-over-InfiniBand as well as with userspace applications
(in conjunction with InfiniBand userspace access).
For QLogic PCIe QLE based cards, use the QIB driver instead.
+
+ If you have this hardware you will need to boot with PAT disabled
+ on your x86-64 systems, use the nopat kernel parameter.
diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c b/drivers/infiniband/hw/ipath/ipath_driver.c
index bd0caed..441cfe5 100644
--- a/drivers/infiniband/hw/ipath/ipath_driver.c
+++ b/drivers/infiniband/hw/ipath/ipath_driver.c
@@ -42,6 +42,9 @@
#include <linux/bitmap.h>
#include <linux/slab.h>
#include <linux/module.h>
+#ifdef CONFIG_X86_64
+#include <asm/pat.h>
+#endif
#include "ipath_kernel.h"
#include "ipath_verbs.h"
@@ -395,6 +398,14 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
unsigned long long addr;
u32 bar0 = 0, bar1 = 0;
+#ifdef CONFIG_X86_64
+ if (WARN(pat_enabled(),
+ "ipath needs PAT disabled, boot with nopat kernel parameter\n")) {
+ ret = EINVAL;
+ goto bail;
+ }
+#endif
+
dd = ipath_alloc_devdata(pdev);
if (IS_ERR(dd)) {
ret = PTR_ERR(dd);
@@ -542,6 +553,7 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
dd->ipath_kregbase = __ioremap(addr, len,
(_PAGE_NO_CACHE|_PAGE_WRITETHRU));
#else
+ /* XXX: split this properly to enable on PAT */
dd->ipath_kregbase = ioremap_nocache(addr, len);
#endif
@@ -587,12 +599,8 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
ret = ipath_enable_wc(dd);
- if (ret) {
- ipath_dev_err(dd, "Write combining not enabled "
- "(err %d): performance may be poor\n",
- -ret);
+ if (ret)
ret = 0;
- }
ipath_verify_pioperf(dd);
diff --git a/drivers/infiniband/hw/ipath/ipath_kernel.h b/drivers/infiniband/hw/ipath/ipath_kernel.h
index e08db70..f0f9471 100644
--- a/drivers/infiniband/hw/ipath/ipath_kernel.h
+++ b/drivers/infiniband/hw/ipath/ipath_kernel.h
@@ -463,9 +463,7 @@ struct ipath_devdata {
/* offset in HT config space of slave/primary interface block */
u8 ipath_ht_slave_off;
/* for write combining settings */
- unsigned long ipath_wc_cookie;
- unsigned long ipath_wc_base;
- unsigned long ipath_wc_len;
+ int wc_cookie;
/* ref count for each pkey */
atomic_t ipath_pkeyrefs[4];
/* shadow copy of struct page *'s for exp tid pages */
diff --git a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
index 70c1f3a..7b6e4c8 100644
--- a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
+++ b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
@@ -37,7 +37,6 @@
*/
#include <linux/pci.h>
-#include <asm/mtrr.h>
#include <asm/processor.h>
#include "ipath_kernel.h"
@@ -122,27 +121,14 @@ int ipath_enable_wc(struct ipath_devdata *dd)
}
if (!ret) {
- int cookie;
- ipath_cdbg(VERBOSE, "Setting mtrr for chip to WC "
- "(addr %llx, len=0x%llx)\n",
- (unsigned long long) pioaddr,
- (unsigned long long) piolen);
- cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 1);
- if (cookie < 0) {
- {
- dev_info(&dd->pcidev->dev,
- "mtrr_add() WC for PIO bufs "
- "failed (%d)\n",
- cookie);
- ret = -EINVAL;
- }
- } else {
- ipath_cdbg(VERBOSE, "Set mtrr for chip to WC, "
- "cookie is %d\n", cookie);
- dd->ipath_wc_cookie = cookie;
- dd->ipath_wc_base = (unsigned long) pioaddr;
- dd->ipath_wc_len = (unsigned long) piolen;
- }
+ dd->wc_cookie = arch_phys_wc_add(pioaddr, piolen);
+ if (dd->wc_cookie < 0) {
+ ipath_dev_err(dd, "Seting mtrr failed on PIO buffers\n");
+ ret = -ENODEV;
+ } else if (dd->wc_cookie = 0)
+ ipath_cdbg(VERBOSE, "Set mtrr for chip to WC not needed\n");
+ else
+ ipath_cdbg(VERBOSE, "Set mtrr for chip to WC\n");
}
return ret;
@@ -154,16 +140,5 @@ int ipath_enable_wc(struct ipath_devdata *dd)
*/
void ipath_disable_wc(struct ipath_devdata *dd)
{
- if (dd->ipath_wc_cookie) {
- int r;
- ipath_cdbg(VERBOSE, "undoing WCCOMB on pio buffers\n");
- r = mtrr_del(dd->ipath_wc_cookie, dd->ipath_wc_base,
- dd->ipath_wc_len);
- if (r < 0)
- dev_info(&dd->pcidev->dev,
- "mtrr_del(%lx, %lx, %lx) failed: %d\n",
- dd->ipath_wc_cookie, dd->ipath_wc_base,
- dd->ipath_wc_len, r);
- dd->ipath_wc_cookie = 0; /* even on failure */
- }
+ arch_phys_wc_del(dd->wc_cookie);
}
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* Re: [PATCH v6 1/3] ivtv: use arch_phys_wc_add() and require PAT disabled
From: Mauro Carvalho Chehab @ 2015-06-09 0:56 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: bp, tomi.valkeinen, bhelgaas, linux-media, linux-rdma,
linux-kernel, Luis R. Rodriguez, Andy Walls, Doug Ledford,
Andy Lutomirski, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
Juergen Gross, Daniel Vetter, Dave Airlie, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Dave Hansen, Arnd Bergmann,
Michael S. Tsirkin, Stefan Bader, Ville Syrjälä,
Mel Gorman, Vlastimil Babka, Davidlohr Bueso, konrad.wilk,
ville.syrjala, david.vrabel, jbeulich, toshi.kani,
Roger Pau Monné, linux-fbdev, ivtv-devel, xen-devel
In-Reply-To: <1433809222-28261-2-git-send-email-mcgrof@do-not-panic.com>
Em Mon, 08 Jun 2015 17:20:20 -0700
"Luis R. Rodriguez" <mcgrof@do-not-panic.com> escreveu:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> We are burrying direct access to MTRR code support on
> x86 in order to take advantage of PAT. In the future we
> also want to make the default behaviour of ioremap_nocache()
> to use strong UC, use of mtrr_add() on those systems
> would make write-combining void.
>
> In order to help both enable us to later make strong
> UC default and in order to phase out direct MTRR access
> code port the driver over to arch_phys_wc_add() and
> annotate that the device driver requires systems to
> boot with PAT disabled, with the nopat kernel parameter.
>
> This is a worthy comprmise given that the hardware is
> really rare these days, and perhaps only some lost souls
> in some third world country are expected to be using this
> feature of the device driver.
>
> Acked-by: Andy Walls <awalls@md.metrocast.net>
> Cc: Andy Walls <awalls@md.metrocast.net>
> Cc: Doug Ledford <dledford@redhat.com>
> Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Provided that you fix the issues below:
Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Suresh Siddha <sbsiddha@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Stefan Bader <stefan.bader@canonical.com>
> Cc: Ville Syrjälä <syrjala@sci.fi>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Davidlohr Bueso <dbueso@suse.de>
> Cc: konrad.wilk@oracle.com
> Cc: ville.syrjala@linux.intel.com
> Cc: david.vrabel@citrix.com
> Cc: jbeulich@suse.com
> Cc: toshi.kani@hp.com
> Cc: Roger Pau Monné <roger.pau@citrix.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: ivtv-devel@ivtvdriver.org
> Cc: linux-media@vger.kernel.org
> Cc: xen-devel@lists.xensource.com
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
> drivers/media/pci/ivtv/Kconfig | 3 +++
> drivers/media/pci/ivtv/ivtvfb.c | 58 ++++++++++++++++-------------------------
> 2 files changed, 26 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
> index dd6ee57e..b2a7f88 100644
> --- a/drivers/media/pci/ivtv/Kconfig
> +++ b/drivers/media/pci/ivtv/Kconfig
> @@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
> This is used in the Hauppauge PVR-350 card. There is a driver
> homepage at <http://www.ivtvdriver.org>.
>
> + If you have this hardware you will need to boot with PAT disabled
> + on your x86 systems, use the nopat kernel parameter.
> +
Hmm... FB_IVTV is not hardware... it is framebuffer support for IVTV.
It is optional to use FB API for the video output port of this board,
instead of using V4L2 API.
I would say, instead, something like:
"In order to use this module, you will need to boot with PAT disabled
on x86 systems, using the nopat kernel parameter."
> To compile this driver as a module, choose M here: the
> module will be called ivtvfb.
> diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
> index 9ff1230..7685ae3 100644
> --- a/drivers/media/pci/ivtv/ivtvfb.c
> +++ b/drivers/media/pci/ivtv/ivtvfb.c
> @@ -44,8 +44,8 @@
> #include <linux/ivtvfb.h>
> #include <linux/slab.h>
>
> -#ifdef CONFIG_MTRR
> -#include <asm/mtrr.h>
> +#ifdef CONFIG_X86_64
> +#include <asm/pat.h>
> #endif
>
> #include "ivtv-driver.h"
> @@ -155,12 +155,11 @@ struct osd_info {
> /* Buffer size */
> u32 video_buffer_size;
>
> -#ifdef CONFIG_MTRR
> /* video_base rounded down as required by hardware MTRRs */
> unsigned long fb_start_aligned_physaddr;
> /* video_base rounded up as required by hardware MTRRs */
> unsigned long fb_end_aligned_physaddr;
> -#endif
> + int wc_cookie;
>
> /* Store the buffer offset */
> int set_osd_coords_x;
> @@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
> static int ivtvfb_init_io(struct ivtv *itv)
> {
> struct osd_info *oi = itv->osd_info;
> + /* Find the largest power of two that maps the whole buffer */
> + int size_shift = 31;
>
> mutex_lock(&itv->serialize_lock);
> if (ivtv_init_on_first_open(itv)) {
> @@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
> oi->video_pbase, oi->video_vbase,
> oi->video_buffer_size / 1024);
>
> -#ifdef CONFIG_MTRR
> - {
> - /* Find the largest power of two that maps the whole buffer */
> - int size_shift = 31;
> -
> - while (!(oi->video_buffer_size & (1 << size_shift))) {
> - size_shift--;
> - }
> - size_shift++;
> - oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
> - oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
> - oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
> - oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
> - if (mtrr_add(oi->fb_start_aligned_physaddr,
> - oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr,
> - MTRR_TYPE_WRCOMB, 1) < 0) {
> - IVTVFB_INFO("disabled mttr\n");
> - oi->fb_start_aligned_physaddr = 0;
> - oi->fb_end_aligned_physaddr = 0;
> - }
> - }
> -#endif
> -
> + while (!(oi->video_buffer_size & (1 << size_shift)))
> + size_shift--;
> + size_shift++;
> + oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
> + oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
> + oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
> + oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
> + oi->wc_cookie = arch_phys_wc_add(oi->fb_start_aligned_physaddr,
> + oi->fb_end_aligned_physaddr -
> + oi->fb_start_aligned_physaddr);
> /* Blank the entire osd. */
> memset_io(oi->video_vbase, 0, oi->video_buffer_size);
>
> @@ -1172,14 +1160,7 @@ static void ivtvfb_release_buffers (struct ivtv *itv)
>
> /* Release pseudo palette */
> kfree(oi->ivtvfb_info.pseudo_palette);
> -
> -#ifdef CONFIG_MTRR
> - if (oi->fb_end_aligned_physaddr) {
> - mtrr_del(-1, oi->fb_start_aligned_physaddr,
> - oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr);
> - }
> -#endif
> -
> + arch_phys_wc_del(oi->wc_cookie);
> kfree(oi);
> itv->osd_info = NULL;
> }
> @@ -1284,6 +1265,13 @@ static int __init ivtvfb_init(void)
> int registered = 0;
> int err;
>
> +#ifdef CONFIG_X86_64
> + if (WARN(pat_enabled(),
> + "ivtvfb needs PAT disabled, boot with nopat kernel parameter\n")) {
> + return EINVAL;
Errors are always negative. So:
return -EINVAL
Or, perhaps, -ENODEV.
> + }
> +#endif
> +
> if (ivtvfb_card_id < -1 || ivtvfb_card_id >= IVTV_MAX_CARDS) {
> printk(KERN_ERR "ivtvfb: ivtvfb_card_id parameter is out of range (valid range: -1 - %d)\n",
> IVTV_MAX_CARDS - 1);
^ permalink raw reply
* Re: [PATCH v6 1/3] ivtv: use arch_phys_wc_add() and require PAT disabled
From: Hans Verkuil @ 2015-06-09 6:23 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Luis R. Rodriguez
Cc: bp, tomi.valkeinen, bhelgaas, linux-media, linux-rdma,
linux-kernel, Luis R. Rodriguez, Andy Walls, Doug Ledford,
Andy Lutomirski, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
Juergen Gross, Daniel Vetter, Dave Airlie, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Dave Hansen, Arnd Bergmann,
Michael S. Tsirkin, Stefan Bader, Ville Syrjälä,
Mel Gorman, Vlastimil Babka, Davidlohr Bueso, konrad.wilk,
ville.syrjala, david.vrabel, jbeulich, toshi.kani,
Roger Pau Monné, linux-fbdev, ivtv-devel, xen-devel
In-Reply-To: <20150608215625.39544c82@recife.lan>
On 06/09/2015 02:56 AM, Mauro Carvalho Chehab wrote:
> Em Mon, 08 Jun 2015 17:20:20 -0700
> "Luis R. Rodriguez" <mcgrof@do-not-panic.com> escreveu:
>
>> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>>
>> We are burrying direct access to MTRR code support on
>> x86 in order to take advantage of PAT. In the future we
>> also want to make the default behaviour of ioremap_nocache()
>> to use strong UC, use of mtrr_add() on those systems
>> would make write-combining void.
>>
>> In order to help both enable us to later make strong
>> UC default and in order to phase out direct MTRR access
>> code port the driver over to arch_phys_wc_add() and
>> annotate that the device driver requires systems to
>> boot with PAT disabled, with the nopat kernel parameter.
>>
>> This is a worthy comprmise given that the hardware is
>> really rare these days, and perhaps only some lost souls
>> in some third world country are expected to be using this
>> feature of the device driver.
>>
>> Acked-by: Andy Walls <awalls@md.metrocast.net>
>> Cc: Andy Walls <awalls@md.metrocast.net>
>> Cc: Doug Ledford <dledford@redhat.com>
>> Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
> Provided that you fix the issues below:
> Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
>> Cc: Andy Lutomirski <luto@amacapital.net>
>> Cc: Suresh Siddha <sbsiddha@gmail.com>
>> Cc: Ingo Molnar <mingo@elte.hu>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Juergen Gross <jgross@suse.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Dave Airlie <airlied@redhat.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Antonino Daplas <adaplas@gmail.com>
>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: Dave Hansen <dave.hansen@linux.intel.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Stefan Bader <stefan.bader@canonical.com>
>> Cc: Ville Syrjälä <syrjala@sci.fi>
>> Cc: Mel Gorman <mgorman@suse.de>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> Cc: Borislav Petkov <bp@suse.de>
>> Cc: Davidlohr Bueso <dbueso@suse.de>
>> Cc: konrad.wilk@oracle.com
>> Cc: ville.syrjala@linux.intel.com
>> Cc: david.vrabel@citrix.com
>> Cc: jbeulich@suse.com
>> Cc: toshi.kani@hp.com
>> Cc: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: linux-fbdev@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: ivtv-devel@ivtvdriver.org
>> Cc: linux-media@vger.kernel.org
>> Cc: xen-devel@lists.xensource.com
>> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
>> ---
>> drivers/media/pci/ivtv/Kconfig | 3 +++
>> drivers/media/pci/ivtv/ivtvfb.c | 58 ++++++++++++++++-------------------------
>> 2 files changed, 26 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
>> index dd6ee57e..b2a7f88 100644
>> --- a/drivers/media/pci/ivtv/Kconfig
>> +++ b/drivers/media/pci/ivtv/Kconfig
>> @@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
>> This is used in the Hauppauge PVR-350 card. There is a driver
>> homepage at <http://www.ivtvdriver.org>.
>>
>> + If you have this hardware you will need to boot with PAT disabled
>> + on your x86 systems, use the nopat kernel parameter.
>> +
>
> Hmm... FB_IVTV is not hardware... it is framebuffer support for IVTV.
> It is optional to use FB API for the video output port of this board,
> instead of using V4L2 API.
That's not true. It is hardware: it drives the video output OSD which overlays
the video output. The reason it is optional is that it is hard to unload a
framebuffer module and most people don't need it.
So V4L2 drives the video output and ivtvfb drives the OSD overlay. So it is
not a case of 'instead of'.
>
> I would say, instead, something like:
>
> "In order to use this module, you will need to boot with PAT disabled
> on x86 systems, using the nopat kernel parameter."
I do agree with this change, but that's because this module is optional and
not for the reasons you mentioned above.
Regards,
Hans
>
>> To compile this driver as a module, choose M here: the
>> module will be called ivtvfb.
>> diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
>> index 9ff1230..7685ae3 100644
>> --- a/drivers/media/pci/ivtv/ivtvfb.c
>> +++ b/drivers/media/pci/ivtv/ivtvfb.c
>> @@ -44,8 +44,8 @@
>> #include <linux/ivtvfb.h>
>> #include <linux/slab.h>
>>
>> -#ifdef CONFIG_MTRR
>> -#include <asm/mtrr.h>
>> +#ifdef CONFIG_X86_64
>> +#include <asm/pat.h>
>> #endif
>>
>> #include "ivtv-driver.h"
>> @@ -155,12 +155,11 @@ struct osd_info {
>> /* Buffer size */
>> u32 video_buffer_size;
>>
>> -#ifdef CONFIG_MTRR
>> /* video_base rounded down as required by hardware MTRRs */
>> unsigned long fb_start_aligned_physaddr;
>> /* video_base rounded up as required by hardware MTRRs */
>> unsigned long fb_end_aligned_physaddr;
>> -#endif
>> + int wc_cookie;
>>
>> /* Store the buffer offset */
>> int set_osd_coords_x;
>> @@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
>> static int ivtvfb_init_io(struct ivtv *itv)
>> {
>> struct osd_info *oi = itv->osd_info;
>> + /* Find the largest power of two that maps the whole buffer */
>> + int size_shift = 31;
>>
>> mutex_lock(&itv->serialize_lock);
>> if (ivtv_init_on_first_open(itv)) {
>> @@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
>> oi->video_pbase, oi->video_vbase,
>> oi->video_buffer_size / 1024);
>>
>> -#ifdef CONFIG_MTRR
>> - {
>> - /* Find the largest power of two that maps the whole buffer */
>> - int size_shift = 31;
>> -
>> - while (!(oi->video_buffer_size & (1 << size_shift))) {
>> - size_shift--;
>> - }
>> - size_shift++;
>> - oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
>> - oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
>> - oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
>> - oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
>> - if (mtrr_add(oi->fb_start_aligned_physaddr,
>> - oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr,
>> - MTRR_TYPE_WRCOMB, 1) < 0) {
>> - IVTVFB_INFO("disabled mttr\n");
>> - oi->fb_start_aligned_physaddr = 0;
>> - oi->fb_end_aligned_physaddr = 0;
>> - }
>> - }
>> -#endif
>> -
>> + while (!(oi->video_buffer_size & (1 << size_shift)))
>> + size_shift--;
>> + size_shift++;
>> + oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
>> + oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
>> + oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
>> + oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
>> + oi->wc_cookie = arch_phys_wc_add(oi->fb_start_aligned_physaddr,
>> + oi->fb_end_aligned_physaddr -
>> + oi->fb_start_aligned_physaddr);
>> /* Blank the entire osd. */
>> memset_io(oi->video_vbase, 0, oi->video_buffer_size);
>>
>> @@ -1172,14 +1160,7 @@ static void ivtvfb_release_buffers (struct ivtv *itv)
>>
>> /* Release pseudo palette */
>> kfree(oi->ivtvfb_info.pseudo_palette);
>> -
>> -#ifdef CONFIG_MTRR
>> - if (oi->fb_end_aligned_physaddr) {
>> - mtrr_del(-1, oi->fb_start_aligned_physaddr,
>> - oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr);
>> - }
>> -#endif
>> -
>> + arch_phys_wc_del(oi->wc_cookie);
>> kfree(oi);
>> itv->osd_info = NULL;
>> }
>> @@ -1284,6 +1265,13 @@ static int __init ivtvfb_init(void)
>> int registered = 0;
>> int err;
>>
>> +#ifdef CONFIG_X86_64
>> + if (WARN(pat_enabled(),
>> + "ivtvfb needs PAT disabled, boot with nopat kernel parameter\n")) {
>> + return EINVAL;
>
> Errors are always negative. So:
> return -EINVAL
>
> Or, perhaps, -ENODEV.
>
>> + }
>> +#endif
>> +
>> if (ivtvfb_card_id < -1 || ivtvfb_card_id >= IVTV_MAX_CARDS) {
>> printk(KERN_ERR "ivtvfb: ivtvfb_card_id parameter is out of range (valid range: -1 - %d)\n",
>> IVTV_MAX_CARDS - 1);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" 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
* [PATCH] drivers/video/fbdev/sis/sis_main.c: avoid repeated strlen() calls
From: Rasmus Villemoes @ 2015-06-09 13:04 UTC (permalink / raw)
To: Thomas Winischhofer, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen
Cc: Rasmus Villemoes, linux-fbdev, linux-kernel
gcc is not smart enough to realize that strlen(strbuf1) cannot be
changed by the loop body, so it is forced to recompute it, at least in
the branch containing the assignment. Avoid this by using an
equivalent stopping condition.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
drivers/video/fbdev/sis/sis_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/sis/sis_main.c b/drivers/video/fbdev/sis/sis_main.c
index fcf610edf217..ea95f8bf3922 100644
--- a/drivers/video/fbdev/sis/sis_main.c
+++ b/drivers/video/fbdev/sis/sis_main.c
@@ -172,7 +172,7 @@ static void sisfb_search_mode(char *name, bool quiet)
if(strlen(name) <= 19) {
strcpy(strbuf1, name);
- for(i = 0; i < strlen(strbuf1); i++) {
+ for(i = 0; strbuf1[i]; i++) {
if(strbuf1[i] < '0' || strbuf1[i] > '9') strbuf1[i] = ' ';
}
--
2.1.3
^ permalink raw reply related
* Re: [PATCH 00/21] On-demand device registration
From: Linus Walleij @ 2015-06-10 7:30 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: Mark Rutland, devicetree@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-samsung-soc, dmaengine,
linux-gpio@vger.kernel.org, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
linux-clk, linux-tegra@vger.kernel.org, Rob Herring,
open list:DRM PANEL DRIVERS, Grant Likely, Alexander Holler,
Dan Williams, Dmitry Torokhov, linux-usb@vger.kernel.org
In-Reply-To: <CAAObsKCP-C+TY8Wkk6bQzxZeGYL9X01trtjxfaYp--ZnH07C+w@mail.gmail.com>
On Tue, Jun 2, 2015 at 12:14 PM, Tomeu Vizoso
<tomeu.vizoso@collabora.com> wrote:
> On 2 June 2015 at 10:48, Linus Walleij <linus.walleij@linaro.org> wrote:
>> This is what systemd is doing in userspace for starting services:
>> ask for your dependencies and wait for them if they are not
>> there. So drivers ask for resources and wait for them. It also
>> needs to be abstract, so for example we need to be able to
>> hang on regulator_get() until the driver is up and providing that
>> regulator, and as long as everything is in slowpath it should
>> be OK. (And vice versa mutatis mutandis for clk, gpio, pin
>> control, interrupts (!) and DMA channels for example.)
>
> I understood above that you propose probing devices in order, but now
> you mention that resource getters would block until the dependency is
> fulfilled which confuses me because if we are probing in order then
> all dependencies would be fulfilled before the device in question gets
> probed.
Sorry, the problem space is a bit convoluted so the answers
get a bit convoluted. Maybe I'm thinking aloud and altering the course
of my thoughts as I type...
I guess there can be explicit dependencies for resources like this
patch does, but another way would be for all resource fetch functions
to be instrumented, so that you do not block until you try to take
a resource that is not yet there, e.g.:
regulator_get(...) -> not available, so:
- identify target regulator provider - this will need instrumentation
- probe it
It then turns out the regulator driver is on the i2c bus, so we
need to probe the i2c driver:
- identify target i2c host for the regulator driver - this will need
instrumentation
- probe the i2c host driver
i2c host comes out, probes the regulator driver, regulator driver
probes and then the regulator_get() call returns.
This requires instrumentation on anything providing a resource
to another driver like those I mentioned and a lot of overhead
infrastructure, but I think it's the right approach. However I don't
know if I would ever be able to pull that off myself, I know talk
is cheap and I should show the code instead.
Deepest respect for your efforts!
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 00/21] On-demand device registration
From: Tomeu Vizoso @ 2015-06-10 10:19 UTC (permalink / raw)
To: Linus Walleij
Cc: Mark Rutland, devicetree@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-samsung-soc, dmaengine,
linux-gpio@vger.kernel.org, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org,
linux-clk, linux-tegra@vger.kernel.org, Rob Herring,
open list:DRM PANEL DRIVERS, Grant Likely, Alexander Holler,
Dan Williams, Dmitry Torokhov, linux-usb@vger.kernel.org
In-Reply-To: <CACRpkdZuSj+S2x1yz6n5PgcRK=3ZCcq_To124b2Y+Oj54aydpg@mail.gmail.com>
On 10 June 2015 at 09:30, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Jun 2, 2015 at 12:14 PM, Tomeu Vizoso
> <tomeu.vizoso@collabora.com> wrote:
>> On 2 June 2015 at 10:48, Linus Walleij <linus.walleij@linaro.org> wrote:
>
>>> This is what systemd is doing in userspace for starting services:
>>> ask for your dependencies and wait for them if they are not
>>> there. So drivers ask for resources and wait for them. It also
>>> needs to be abstract, so for example we need to be able to
>>> hang on regulator_get() until the driver is up and providing that
>>> regulator, and as long as everything is in slowpath it should
>>> be OK. (And vice versa mutatis mutandis for clk, gpio, pin
>>> control, interrupts (!) and DMA channels for example.)
>>
>> I understood above that you propose probing devices in order, but now
>> you mention that resource getters would block until the dependency is
>> fulfilled which confuses me because if we are probing in order then
>> all dependencies would be fulfilled before the device in question gets
>> probed.
>
> Sorry, the problem space is a bit convoluted so the answers
> get a bit convoluted. Maybe I'm thinking aloud and altering the course
> of my thoughts as I type...
>
> I guess there can be explicit dependencies for resources like this
> patch does, but another way would be for all resource fetch functions
> to be instrumented, so that you do not block until you try to take
> a resource that is not yet there, e.g.:
>
> regulator_get(...) -> not available, so:
> - identify target regulator provider - this will need instrumentation
> - probe it
>
> It then turns out the regulator driver is on the i2c bus, so we
> need to probe the i2c driver:
> - identify target i2c host for the regulator driver - this will need
> instrumentation
> - probe the i2c host driver
>
> i2c host comes out, probes the regulator driver, regulator driver
> probes and then the regulator_get() call returns.
Hmm, if I understand correctly what you say, this is exactly what this
particular series does:
regulator_get -> of_platform_device_ensure -> probe() on the platform
device that encloses the requested device node (i2c host) -> i2c slave
gets probed and the regulator registered -> regulator_get returns the
requested resource
The downside I'm currently looking at is that an explicit dependency
graph would be useful to have for other purposes. For example to print
a neat warning when a dependency cannot be fulfilled. Or to refuse to
unbind a device which other devices depend on, or to automatically
unbind the devices that depend on it, or to print a warning if a
device is hotplugged off and other devices depend on it.
> This requires instrumentation on anything providing a resource
> to another driver like those I mentioned and a lot of overhead
> infrastructure, but I think it's the right approach. However I don't
> know if I would ever be able to pull that off myself, I know talk
> is cheap and I should show the code instead.
Yeah, if you can give it a second look and say if it matches what you
wrote above, it would be very much appreciated.
> Deepest respect for your efforts!
Thanks!
Tomeu
> Yours,
> Linus Walleij
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 00/21] On-demand device registration
From: Andrzej Hajda @ 2015-06-10 12:23 UTC (permalink / raw)
To: Tomeu Vizoso, Linus Walleij
Cc: Mark Rutland, devicetree@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-samsung-soc,
linux-tegra@vger.kernel.org, linux-pm@vger.kernel.org,
Dmitry Torokhov, linux-kernel@vger.kernel.org,
linux-pwm@vger.kernel.org, linux-gpio@vger.kernel.org,
Rob Herring, open list:DRM PANEL DRIVERS, dmaengine,
Alexander Holler, Dan Williams, linux-usb, linux-clk,
Grant Likely
In-Reply-To: <CAAObsKD97nEARO5eZYA2g8+z_MrazBy-m8PV_kSyj19ya72mAw@mail.gmail.com>
On 06/10/2015 12:19 PM, Tomeu Vizoso wrote:
> On 10 June 2015 at 09:30, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Tue, Jun 2, 2015 at 12:14 PM, Tomeu Vizoso
>> <tomeu.vizoso@collabora.com> wrote:
>>> On 2 June 2015 at 10:48, Linus Walleij <linus.walleij@linaro.org> wrote:
>>
>>>> This is what systemd is doing in userspace for starting services:
>>>> ask for your dependencies and wait for them if they are not
>>>> there. So drivers ask for resources and wait for them. It also
>>>> needs to be abstract, so for example we need to be able to
>>>> hang on regulator_get() until the driver is up and providing that
>>>> regulator, and as long as everything is in slowpath it should
>>>> be OK. (And vice versa mutatis mutandis for clk, gpio, pin
>>>> control, interrupts (!) and DMA channels for example.)
>>>
>>> I understood above that you propose probing devices in order, but now
>>> you mention that resource getters would block until the dependency is
>>> fulfilled which confuses me because if we are probing in order then
>>> all dependencies would be fulfilled before the device in question gets
>>> probed.
>>
>> Sorry, the problem space is a bit convoluted so the answers
>> get a bit convoluted. Maybe I'm thinking aloud and altering the course
>> of my thoughts as I type...
>>
>> I guess there can be explicit dependencies for resources like this
>> patch does, but another way would be for all resource fetch functions
>> to be instrumented, so that you do not block until you try to take
>> a resource that is not yet there, e.g.:
>>
>> regulator_get(...) -> not available, so:
>> - identify target regulator provider - this will need instrumentation
>> - probe it
>>
>> It then turns out the regulator driver is on the i2c bus, so we
>> need to probe the i2c driver:
>> - identify target i2c host for the regulator driver - this will need
>> instrumentation
>> - probe the i2c host driver
>>
>> i2c host comes out, probes the regulator driver, regulator driver
>> probes and then the regulator_get() call returns.
>
> Hmm, if I understand correctly what you say, this is exactly what this
> particular series does:
>
> regulator_get -> of_platform_device_ensure -> probe() on the platform
> device that encloses the requested device node (i2c host) -> i2c slave
> gets probed and the regulator registered -> regulator_get returns the
> requested resource
The downside of this solution is that it will not work without device
tree or even without device dependencies not explicitly specified in
device tree.
>
> The downside I'm currently looking at is that an explicit dependency
> graph would be useful to have for other purposes. For example to print
> a neat warning when a dependency cannot be fulfilled. Or to refuse to
> unbind a device which other devices depend on,
As I understand Greg you cannot prevent unbinding by design, see [1].
[1]: http://thread.gmane.org/gmane.linux.kernel/1154308/focus\x1154648
> or to automatically
> unbind the devices that depend on it,
What about devices that have weak dependency? They should not be unbound
but they should be somehow noticed about unbinding.
In general many kernel frameworks are broken in handling hot-unbinding
of drivers, consumers are not noticed about unbinding of their resource
providers and usually they stay with broken handles or handles to dummy
resources.
I suspect the only proper solution for handling resources that can
dynamically appear/disappear is to provide notification to their
consumers about appearance change of the resource.
I have proposed some times ago solution for above problems based on the
statement above, cover letter explains it in more detail [2].
In short it solves following issues:
- consumer receives resource as soon as it becomes available,
- consumer is notified just before resource removal,
- it can properly handle provider unbind/re-bind,
- it avoids late init due to deferred probing,
- it allows to track optional resources.
[2]: http://thread.gmane.org/gmane.linux.kernel.gpio/5201
Regards
Andrzej
> or to print a warning if a
> device is hotplugged off and other devices depend on it.
>
>> This requires instrumentation on anything providing a resource
>> to another driver like those I mentioned and a lot of overhead
>> infrastructure, but I think it's the right approach. However I don't
>> know if I would ever be able to pull that off myself, I know talk
>> is cheap and I should show the code instead.
>
> Yeah, if you can give it a second look and say if it matches what you
> wrote above, it would be very much appreciated.
>
>> Deepest respect for your efforts!
>
> Thanks!
>
> Tomeu
>
>> Yours,
>> Linus Walleij
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> --
> To unsubscribe from this list: send the line "unsubscribe linux-gpio" 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
* Klientskie bazi tel +79133913837 Yznaite podrobnee!!!
From: Klientskie bazi tel +79133913837 Yznaite podrobnee!!! @ 2015-06-10 12:53 UTC (permalink / raw)
To: linux-fbdev
Klientskie bazi tel +79133913837 Yznaite podrobnee!!!
^ permalink raw reply
* re: video, sm501: add OF binding to support SM501
From: Dan Carpenter @ 2015-06-10 15:51 UTC (permalink / raw)
To: linux-fbdev
Hello Heiko Schocher,
The patch 4295f9bf74a8: "video, sm501: add OF binding to support
SM501" from Jan 26, 2011, leads to the following static checker
warning:
drivers/video/fbdev/sm501fb.c:1958 sm501fb_probe()
warn: strcpy() 'cp' of unknown size might be too large for 'fb_mode'
drivers/video/fbdev/sm501fb.c
46 static char *fb_mode = "640x480-16@60";
[ snip ]
1953 info->pdata = &sm501fb_def_pdata;
1954 if (np) {
1955 /* Get EDID */
1956 cp = of_get_property(np, "mode", &len);
1957 if (cp)
1958 strcpy(fb_mode, cp);
I don't know anything about this hardware but there might be a mode
longer than "640x480-16@60"?
1959 prop = of_get_property(np, "edid", &len);
1960 if (prop && len = EDID_LENGTH) {
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH 00/21] On-demand device registration
From: Linus Walleij @ 2015-06-11 8:12 UTC (permalink / raw)
To: Alexander Holler
Cc: Tomeu Vizoso, Grant Likely, Mark Rutland,
devicetree@vger.kernel.org, linux-fbdev@vger.kernel.org,
linux-samsung-soc, linux-tegra@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-pm@vger.kernel.org,
Dmitry Torokhov, linux-kernel@vger.kernel.org, Rob Herring,
linux-pwm@vger.kernel.org, open,
list@wandq.ahsoftware:DRM PANEL DRIVERS <dri-devel@lists.freedesktop.org>, dmaengine@vger.kernel.org, Dan Williams <dan.j.williams@intel.com>, linux-usb@vger.kernel.org
In-Reply-To: <5577F533.1060007@ahsoftware.de>
On Wed, Jun 10, 2015 at 10:28 AM, Alexander Holler <holler@ahsoftware.de> wrote:
> Am 10.06.2015 um 09:30 schrieb Linus Walleij:
>> i2c host comes out, probes the regulator driver, regulator driver
>> probes and then the regulator_get() call returns.
>>
>> This requires instrumentation on anything providing a resource
>> to another driver like those I mentioned and a lot of overhead
>> infrastructure, but I think it's the right approach. However I don't
>> know if I would ever be able to pull that off myself, I know talk
>> is cheap and I should show the code instead.
>
> You would end up with the same problem of deadlocks as currently, and you
> would still need something ugly like the defered probe brutforce to avoid
> them.
Sorry I don't get that. Care to elaborate on why?
Yours,
Linus Walleij
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox