* [RESEND PATCH] staging: sm750fb: cleanup: fix brace placement
From: Charles Rose @ 2016-01-24 22:36 UTC (permalink / raw)
To: linux-fbdev
Fix brace placement warnings caught by checkpatch.pl.
Signed-off-by: Charles Rose <charles.rose.linux@gmail.com>
---
This is a resend of the original patch posted on 20160119 where my
recepient list was incomplete.
drivers/staging/sm750fb/sm750_cursor.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750_cursor.c b/drivers/staging/sm750fb/sm750_cursor.c
index 3b7ce92..cab891c 100644
--- a/drivers/staging/sm750fb/sm750_cursor.c
+++ b/drivers/staging/sm750fb/sm750_cursor.c
@@ -143,8 +143,7 @@ void hw_cursor_setData(struct lynx_cursor *cursor,
iowrite16(data, pbuffer);
/* assume pitch is 1,2,4,8,...*/
- if ((i+1) % pitch = 0)
- {
+ if ((i+1) % pitch = 0) {
/* need a return */
pstart += offset;
pbuffer = pstart;
--
2.5.0
^ permalink raw reply related
* [PATCH] staging: sm750fb, fix typos
From: Jiri Slaby @ 2016-01-25 15:02 UTC (permalink / raw)
To: sudipm.mukherjee
Cc: linux-kernel, Matej Vasek, Teddy Wang, Greg Kroah-Hartman,
linux-fbdev, devel, Jiri Slaby
From: Matej Vasek <xvasek1@fi.muni.cz>
The code contained typos like "structur", "fointers", etc. Fix that.
No code change, only comments.
Signed-off-by: Matej Vasek <xvasek1@fi.muni.cz>
Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Cc: Teddy Wang <teddy.wang@siliconmotion.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: <linux-fbdev@vger.kernel.org>
Cc: <devel@driverdev.osuosl.org>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/staging/sm750fb/ddk750_hwi2c.c | 4 ++--
drivers/staging/sm750fb/ddk750_mode.c | 2 +-
drivers/staging/sm750fb/ddk750_sii164.h | 5 ++++-
drivers/staging/sm750fb/sm750.c | 2 +-
drivers/staging/sm750fb/sm750.h | 2 +-
drivers/staging/sm750fb/sm750_accel.c | 4 ++--
drivers/staging/sm750fb/sm750_accel.h | 2 +-
drivers/staging/sm750fb/sm750_hw.c | 2 +-
8 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_hwi2c.c b/drivers/staging/sm750fb/ddk750_hwi2c.c
index 7be2111284f4..29140949fde7 100644
--- a/drivers/staging/sm750fb/ddk750_hwi2c.c
+++ b/drivers/staging/sm750fb/ddk750_hwi2c.c
@@ -128,7 +128,7 @@ static unsigned int hw_i2c_write_data(
if (hw_i2c_wait_tx_done() != 0)
break;
- /* Substract length */
+ /* Subtract length */
length -= (count + 1);
/* Total byte written */
@@ -195,7 +195,7 @@ static unsigned int hw_i2c_read_data(
for (i = 0; i <= count; i++)
*buf++ = PEEK32(I2C_DATA0 + i);
- /* Substract length by 16 */
+ /* Subtract length by 16 */
length -= (count + 1);
/* Number of bytes read. */
diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c
index fa35926680ab..58421898c83d 100644
--- a/drivers/staging/sm750fb/ddk750_mode.c
+++ b/drivers/staging/sm750fb/ddk750_mode.c
@@ -42,7 +42,7 @@ static unsigned long displayControlAdjust_SM750LE(mode_parameter_t *pModeParam,
dispControl &= FIELD_CLEAR(CRT_DISPLAY_CTRL, CLK);
/* Set bit 29:27 of display control register for the right clock */
- /* Note that SM750LE only need to supported 7 resoluitons. */
+ /* Note that SM750LE only need to supported 7 resolutions. */
if (x = 800 && y = 600)
dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL41);
else if (x = 1024 && y = 768)
diff --git a/drivers/staging/sm750fb/ddk750_sii164.h b/drivers/staging/sm750fb/ddk750_sii164.h
index f2610c90eeb4..664ad089f753 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.h
+++ b/drivers/staging/sm750fb/ddk750_sii164.h
@@ -39,7 +39,10 @@ unsigned char sii164IsConnected(void);
unsigned char sii164CheckInterrupt(void);
void sii164ClearInterrupt(void);
#endif
-/* below register definination is used for Silicon Image SiI164 DVI controller chip */
+/*
+ * below register definition is used for
+ * Silicon Image SiI164 DVI controller chip
+ */
/*
* Vendor ID registers
*/
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index c78421b5b0e7..9dac9ec95428 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -319,7 +319,7 @@ static int lynxfb_ops_set_par(struct fb_info *info)
var = &info->var;
fix = &info->fix;
- /* fix structur is not so FIX ... */
+ /* fix structure is not so FIX ... */
line_length = var->xres_virtual * var->bits_per_pixel / 8;
line_length = ALIGN(line_length, crtc->line_pad);
fix->line_length = line_length;
diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index b0a93cdc7292..b2089542862e 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -53,7 +53,7 @@ struct lynx_accel {
/* base virtual address of de data port */
volatile unsigned char __iomem *dpPortBase;
- /* function fointers */
+ /* function pointers */
void (*de_init)(struct lynx_accel *);
int (*de_wait)(void);/* see if hardware ready to work */
diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index 43e59725920c..e150680a8cd1 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -53,7 +53,7 @@ void hw_de_init(struct lynx_accel *accel)
FIELD_CLEAR(DE_STRETCH_FORMAT, ADDRESSING)&
FIELD_CLEAR(DE_STRETCH_FORMAT, SOURCE_HEIGHT);
- /* DE_STRETCH bpp format need be initilized in setMode routine */
+ /* DE_STRETCH bpp format need be initialized in setMode routine */
write_dpr(accel, DE_STRETCH_FORMAT, (read_dpr(accel, DE_STRETCH_FORMAT) & clr) | reg);
/* disable clipping and transparent */
@@ -305,7 +305,7 @@ int hw_imageblit(struct lynx_accel *accel,
u32 dx,
u32 dy, /* Starting coordinate of destination surface */
u32 width,
- u32 height, /* width and height of rectange in pixel value */
+ u32 height, /* width and height of rectangle in pixel value */
u32 fColor, /* Foreground color (corresponding to a 1 in the monochrome data */
u32 bColor, /* Background color (corresponding to a 0 in the monochrome data */
u32 rop2) /* ROP value */
diff --git a/drivers/staging/sm750fb/sm750_accel.h b/drivers/staging/sm750fb/sm750_accel.h
index f252e47d5ee9..1ec66d28f456 100644
--- a/drivers/staging/sm750fb/sm750_accel.h
+++ b/drivers/staging/sm750fb/sm750_accel.h
@@ -268,7 +268,7 @@ int hw_imageblit(struct lynx_accel *accel,
u32 dx,
u32 dy, /* Starting coordinate of destination surface */
u32 width,
- u32 height, /* width and height of rectange in pixel value */
+ u32 height, /* width and height of rectangle in pixel value */
u32 fColor, /* Foreground color (corresponding to a 1 in the monochrome data */
u32 bColor, /* Background color (corresponding to a 0 in the monochrome data */
u32 rop2);
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index 41822c6c0380..249c71dce657 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -147,7 +147,7 @@ int hw_sm750_inithw(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
break;
}
} else {
- /* for 750LE ,no DVI chip initilization makes Monitor no signal */
+ /* for 750LE ,no DVI chip initialization makes Monitor no signal */
/* Set up GPIO for software I2C to program DVI chip in the
Xilinx SP605 board, in order to have video signal.
*/
--
2.7.0
^ permalink raw reply related
* [PATCH] fbdev: mmp: print IRQ resource using %pR format string
From: Arnd Bergmann @ 2016-01-25 15:03 UTC (permalink / raw)
To: linux-arm-kernel
resource_size_t cannot be printed using the %x format string
when we it is defined as u64:
drivers/video/fbdev/mmp/hw/mmp_ctrl.c: In function 'mmphw_probe':
drivers/video/fbdev/mmp/hw/mmp_ctrl.c:506:22: error: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'resource_size_t {aka long long unsigned int}' [-Werror=format=]
dev_err(ctrl->dev, "%s: res %x - %x map failed\n", __func__,
^
drivers/video/fbdev/mmp/hw/mmp_ctrl.c:506:22: error: format '%x' expects argument of type 'unsigned int', but argument 5 has type 'resource_size_t {aka long long unsigned int}' [-Werror=format=]
This changes the format string to %pR, which is interpreted
by the printk implementation to pretty-print a resource
structure.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/video/fbdev/mmp/hw/mmp_ctrl.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/mmp/hw/mmp_ctrl.c b/drivers/video/fbdev/mmp/hw/mmp_ctrl.c
index de54a4748065..b6f83d5df9fd 100644
--- a/drivers/video/fbdev/mmp/hw/mmp_ctrl.c
+++ b/drivers/video/fbdev/mmp/hw/mmp_ctrl.c
@@ -503,8 +503,7 @@ static int mmphw_probe(struct platform_device *pdev)
ctrl->reg_base = devm_ioremap_nocache(ctrl->dev,
res->start, resource_size(res));
if (ctrl->reg_base = NULL) {
- dev_err(ctrl->dev, "%s: res %x - %x map failed\n", __func__,
- res->start, res->end);
+ dev_err(ctrl->dev, "%s: res %pR map failed\n", __func__, res);
ret = -ENOMEM;
goto failed;
}
--
2.7.0
^ permalink raw reply related
* [PATCH] fbdev: s6e8ax0: avoid unused function warnings
From: Arnd Bergmann @ 2016-01-25 15:04 UTC (permalink / raw)
To: linux-arm-kernel
The s6e8ax0 suspend/resume functions are hidden inside of an #ifdef
when CONFIG_PM is set to avoid unused function warnings, but they
call some other functions that nothing else calls, and we get warnings
about those:
drivers/video/fbdev/exynos/s6e8ax0.c:449:13: error: 's6e8ax0_sleep_in' defined but not used [-Werror=unused-function]
drivers/video/fbdev/exynos/s6e8ax0.c:485:13: error: 's6e8ax0_display_off' defined but not used [-Werror=unused-function]
This marks the PM functions as __maybe_unused so the compiler can
silently drop them when they are not referenced.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/video/fbdev/exynos/s6e8ax0.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/video/fbdev/exynos/s6e8ax0.c b/drivers/video/fbdev/exynos/s6e8ax0.c
index 95873f26e39c..de2f3e793786 100644
--- a/drivers/video/fbdev/exynos/s6e8ax0.c
+++ b/drivers/video/fbdev/exynos/s6e8ax0.c
@@ -829,8 +829,7 @@ static int s6e8ax0_probe(struct mipi_dsim_lcd_device *dsim_dev)
return 0;
}
-#ifdef CONFIG_PM
-static int s6e8ax0_suspend(struct mipi_dsim_lcd_device *dsim_dev)
+static int __maybe_unused s6e8ax0_suspend(struct mipi_dsim_lcd_device *dsim_dev)
{
struct s6e8ax0 *lcd = dev_get_drvdata(&dsim_dev->dev);
@@ -843,7 +842,7 @@ static int s6e8ax0_suspend(struct mipi_dsim_lcd_device *dsim_dev)
return 0;
}
-static int s6e8ax0_resume(struct mipi_dsim_lcd_device *dsim_dev)
+static int __maybe_unused s6e8ax0_resume(struct mipi_dsim_lcd_device *dsim_dev)
{
struct s6e8ax0 *lcd = dev_get_drvdata(&dsim_dev->dev);
@@ -855,10 +854,6 @@ static int s6e8ax0_resume(struct mipi_dsim_lcd_device *dsim_dev)
return 0;
}
-#else
-#define s6e8ax0_suspend NULL
-#define s6e8ax0_resume NULL
-#endif
static struct mipi_dsim_lcd_driver s6e8ax0_dsim_ddi_driver = {
.name = "s6e8ax0",
@@ -867,8 +862,8 @@ static struct mipi_dsim_lcd_driver s6e8ax0_dsim_ddi_driver = {
.power_on = s6e8ax0_power_on,
.set_sequence = s6e8ax0_set_sequence,
.probe = s6e8ax0_probe,
- .suspend = s6e8ax0_suspend,
- .resume = s6e8ax0_resume,
+ .suspend = IS_ENABLED(CONFIG_PM) ? s6e8ax0_suspend : NULL,
+ .resume = IS_ENABLED(CONFIG_PM) ? s6e8ax0_resume : NULL,
};
static int s6e8ax0_init(void)
--
2.7.0
^ permalink raw reply related
* [PATCH] fbdev: da8xx-fb: remove incorrect type cast
From: Arnd Bergmann @ 2016-01-25 15:05 UTC (permalink / raw)
To: linux-arm-kernel
The probe function correct passes a dma_addr_t pointer into
dma_alloc_coherent(), but has a cast to resource_size_t, which
might be different from dma_addr_t:
drivers/video/fbdev/da8xx-fb.c: In function 'fb_probe':
drivers/video/fbdev/da8xx-fb.c:1431:10: error: passing argument 3 of 'dma_alloc_coherent' from incompatible pointer type [-Werror=incompatible-pointer-types]
This removes the cast, which avoids the warning.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/video/fbdev/da8xx-fb.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbdev/da8xx-fb.c b/drivers/video/fbdev/da8xx-fb.c
index 0081725c6b5b..6b2a06d09f2b 100644
--- a/drivers/video/fbdev/da8xx-fb.c
+++ b/drivers/video/fbdev/da8xx-fb.c
@@ -152,7 +152,7 @@ static void lcdc_write(unsigned int val, unsigned int addr)
struct da8xx_fb_par {
struct device *dev;
- resource_size_t p_palette_base;
+ dma_addr_t p_palette_base;
unsigned char *v_palette_base;
dma_addr_t vram_phys;
unsigned long vram_size;
@@ -1428,7 +1428,7 @@ static int fb_probe(struct platform_device *device)
par->vram_virt = dma_alloc_coherent(NULL,
par->vram_size,
- (resource_size_t *) &par->vram_phys,
+ &par->vram_phys,
GFP_KERNEL | GFP_DMA);
if (!par->vram_virt) {
dev_err(&device->dev,
@@ -1448,7 +1448,7 @@ static int fb_probe(struct platform_device *device)
/* allocate palette buffer */
par->v_palette_base = dma_zalloc_coherent(NULL, PALETTE_SIZE,
- (resource_size_t *)&par->p_palette_base,
+ &par->p_palette_base,
GFP_KERNEL | GFP_DMA);
if (!par->v_palette_base) {
dev_err(&device->dev,
--
2.7.0
^ permalink raw reply related
* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Doug Anderson @ 2016-01-25 16:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20151110173416.GB21727@ulmo>
Thierry and Boris,
On Tue, Nov 10, 2015 at 9:34 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Mon, Oct 19, 2015 at 12:12:12PM +0200, Heiko Stübner wrote:
>> Hi Thierry,
>>
>> Am Montag, 21. September 2015, 11:33:17 schrieb Boris Brezillon:
>> > Hello,
>> >
>> > This series adds support for atomic PWM update, or IOW, the capability
>> > to update all the parameters of a PWM device (enabled/disabled, period,
>> > duty and polarity) in one go.
>>
>> is anything more blocking this series? It's now sitting on the lists for
>> nearly a month and everybody seems happy with it, so it would be really nice
>> to have in mainline :-) .
>>
>> Especially as this also makes it possible for Rockchip Chromebooks to actually
>> control the logic-regulator that is implemented as pwm-regulator there.
>
> Last time I tried to put this into linux-next I got immediately
> bombarded by a number of build failures, so I backed things out. The
> current plan is to give this another try after v4.4-rc1.
We're now into the 4.5 timeframe. Does anyone have a concrete set of
things that need to happen before this patch series makes it into
mainline?
From searching I see that the latest version of this series is v4 and
there are a smattering of comments on the 24-patch series. Presumably
a v5 needs to be posted to address those things.
...but it looks like the big sticking point is that Boris is waiting
for a response to his questions in
<https://patchwork.kernel.org/patch/7622881/>. Thierry: can you give
Boris some direction for what else he needs to do? We need to come up
with _some_ solution since this series gets us much better support for
PWM regulators. Without this series or some other solution, PWM
regulators aren't usable in mainline on any system that uses them for
system-critical rails. Nearly all Rockchip reference boards and
shipping devices uses a PWM regulator for the system-critidal "logic"
rail. That means any patches which need to change this rail in Linux
are blocked.
If there's already been off-list discussion and Boris already knows
what the next steps are then my apologies and I'll wait patiently for
the next series. ;)
Thanks!
-Doug
^ permalink raw reply
* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Thierry Reding @ 2016-01-25 17:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=WL3jYEgVe-m6QcAyQwfJgdX4J5OtKisKwyoKOrTSpEQA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6159 bytes --]
On Mon, Jan 25, 2016 at 08:28:31AM -0800, Doug Anderson wrote:
> Thierry and Boris,
>
> On Tue, Nov 10, 2015 at 9:34 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Mon, Oct 19, 2015 at 12:12:12PM +0200, Heiko Stübner wrote:
> >> Hi Thierry,
> >>
> >> Am Montag, 21. September 2015, 11:33:17 schrieb Boris Brezillon:
> >> > Hello,
> >> >
> >> > This series adds support for atomic PWM update, or IOW, the capability
> >> > to update all the parameters of a PWM device (enabled/disabled, period,
> >> > duty and polarity) in one go.
> >>
> >> is anything more blocking this series? It's now sitting on the lists for
> >> nearly a month and everybody seems happy with it, so it would be really nice
> >> to have in mainline :-) .
> >>
> >> Especially as this also makes it possible for Rockchip Chromebooks to actually
> >> control the logic-regulator that is implemented as pwm-regulator there.
> >
> > Last time I tried to put this into linux-next I got immediately
> > bombarded by a number of build failures, so I backed things out. The
> > current plan is to give this another try after v4.4-rc1.
>
> We're now into the 4.5 timeframe. Does anyone have a concrete set of
> things that need to happen before this patch series makes it into
> mainline?
I think the current status is that we're more or less blocked on the
decision on what the reset state of the PWM should be. The question is
what to do if the PWM hardware readout differs from the settings found
in DT.
> From searching I see that the latest version of this series is v4 and
> there are a smattering of comments on the 24-patch series. Presumably
> a v5 needs to be posted to address those things.
>
> ...but it looks like the big sticking point is that Boris is waiting
> for a response to his questions in
> <https://patchwork.kernel.org/patch/7622881/>. Thierry: can you give
> Boris some direction for what else he needs to do? We need to come up
> with _some_ solution since this series gets us much better support for
> PWM regulators. Without this series or some other solution, PWM
> regulators aren't usable in mainline on any system that uses them for
> system-critical rails. Nearly all Rockchip reference boards and
> shipping devices uses a PWM regulator for the system-critidal "logic"
> rail. That means any patches which need to change this rail in Linux
> are blocked.
I really don't understand this design decision. I presume that the PWM
controlling this system-critical logic is driven by the SoC? So if the
regulator is system-critical, doesn't that make it a chicken and egg
problem? How can the SoC turn the PWM on if it doesn't have power? But
perhaps I'm completely misunderstanding what you're saying. Perhaps if
somebody could summarize how exactly this works, it would help better
understand the requirements or what's the correct thing to do.
> If there's already been off-list discussion and Boris already knows
> what the next steps are then my apologies and I'll wait patiently for
> the next series. ;)
I don't think we reached a conclusion on this. And to be honest, I'm not
sure what the right way forward is in this situation. So in order to
make some forward progress I suggest we start a discussion, hopefully
that will clarify the situation and help lead to the conclusion. Let me
recap where we are:
Boris' series has two goals: 1) allow seamless hand-off from firmware to
kernel of a PWM channel and 2) apply changes to a regulator in a single
atomic operation. To achieve this the concept of PWM state is introduced
which encapsulates the settings of a PWM channel. On driver probe the
current state will be read from hardware and when one or more parameters
are to be changed, the current state is duplicated, the new values set
in the state and the new state applied.
The problem that we've encountered is that since the PWM parameters are
specified in DT (or board files), there is the possibility of the PWM
hardware state and the board parameters disagreeing. To resolve such
situations there must be a point in time where both hardware state and
software state must be synchronized. Now the most straightforward way to
do that would be to simply apply the software state and be done with it.
However the software state initially lacks the duty cycle because it is
a parameter that usually depends on the use-case (for backlight for
instance it controls the brightness, for regulators it controls the
output voltage, ...).
Applying the software state as-is also means that there's no reason at
all to read out the hardware state in the first place, because it will
simply be discarded.
An alternative would be to discard the software state and trust the
hardware to be configured correctly. That's somewhat risky because we
don't know if the hardware is properly configured. Or Linux might have
different requirements from the firmware and hence needs to configure
the PWM differently.
Neither of the above are very attractive options. The best I've been
able to come up with so far is to completely remove this decision from
the PWM subsystem and let users handle this. That is, a PWM regulator
driver would have to have all the knowledge about how to configure the
PWM for its needs. So upon probe, the PWM regulator driver would inspect
the current state of the PWM and adjust if necessary, then apply again.
Ideally of course it wouldn't have to do anything because the hardware
PWM state would match the software configuration. The idea here is that
the PWM regulator driver knows exactly what duty cycle to configure to
obtain the desired output voltage.
That doesn't really get us closer, though. There is still the issue of
the user having to deal with two states: the current hardware state and
the software state as configured in DT or board files.
Like I said, I'm on the fence about this, so I'd appreciate any comments
and perhaps insight from user subsystem maintainers on how they'd like
this to look, or how this has been done with other resources (GPIOs,
...?)
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Boris Brezillon @ 2016-01-25 17:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160125170855.GA10182@ulmo>
Hi Thierry,
On Mon, 25 Jan 2016 18:08:55 +0100
Thierry Reding <thierry.reding@gmail.com> wrote:
> On Mon, Jan 25, 2016 at 08:28:31AM -0800, Doug Anderson wrote:
> > Thierry and Boris,
> >
> > On Tue, Nov 10, 2015 at 9:34 AM, Thierry Reding
> > <thierry.reding@gmail.com> wrote:
> > > On Mon, Oct 19, 2015 at 12:12:12PM +0200, Heiko Stübner wrote:
> > >> Hi Thierry,
> > >>
> > >> Am Montag, 21. September 2015, 11:33:17 schrieb Boris Brezillon:
> > >> > Hello,
> > >> >
> > >> > This series adds support for atomic PWM update, or IOW, the capability
> > >> > to update all the parameters of a PWM device (enabled/disabled, period,
> > >> > duty and polarity) in one go.
> > >>
> > >> is anything more blocking this series? It's now sitting on the lists for
> > >> nearly a month and everybody seems happy with it, so it would be really nice
> > >> to have in mainline :-) .
> > >>
> > >> Especially as this also makes it possible for Rockchip Chromebooks to actually
> > >> control the logic-regulator that is implemented as pwm-regulator there.
> > >
> > > Last time I tried to put this into linux-next I got immediately
> > > bombarded by a number of build failures, so I backed things out. The
> > > current plan is to give this another try after v4.4-rc1.
> >
> > We're now into the 4.5 timeframe. Does anyone have a concrete set of
> > things that need to happen before this patch series makes it into
> > mainline?
>
> I think the current status is that we're more or less blocked on the
> decision on what the reset state of the PWM should be. The question is
> what to do if the PWM hardware readout differs from the settings found
> in DT.
I think I already explained my PoV regarding the default/reference
setting (or what you later call the software state).
To me, this has never matched the state of the PWM device, it's just a
reference duty-cycle and polarity configuration that PWM users can
decide to apply or not. Currently, there's no mechanism to apply the
reference PWM config when a user request a PWM (which is the only
moment we could be able to apply this config, since reference configs
are per users and not attached to the PWM device itself).
All I'm trying to do in this series is teach some PWM users to make use
of hardware read-out state instead of blindly applying a default config
that can generate glitches in the PWM signal.
>
> > From searching I see that the latest version of this series is v4 and
> > there are a smattering of comments on the 24-patch series. Presumably
> > a v5 needs to be posted to address those things.
> >
> > ...but it looks like the big sticking point is that Boris is waiting
> > for a response to his questions in
> > <https://patchwork.kernel.org/patch/7622881/>. Thierry: can you give
> > Boris some direction for what else he needs to do? We need to come up
> > with _some_ solution since this series gets us much better support for
> > PWM regulators. Without this series or some other solution, PWM
> > regulators aren't usable in mainline on any system that uses them for
> > system-critical rails. Nearly all Rockchip reference boards and
> > shipping devices uses a PWM regulator for the system-critidal "logic"
> > rail. That means any patches which need to change this rail in Linux
> > are blocked.
>
> I really don't understand this design decision. I presume that the PWM
> controlling this system-critical logic is driven by the SoC? So if the
> regulator is system-critical, doesn't that make it a chicken and egg
> problem? How can the SoC turn the PWM on if it doesn't have power? But
> perhaps I'm completely misunderstanding what you're saying. Perhaps if
> somebody could summarize how exactly this works, it would help better
> understand the requirements or what's the correct thing to do.
>
> > If there's already been off-list discussion and Boris already knows
> > what the next steps are then my apologies and I'll wait patiently for
> > the next series. ;)
>
> I don't think we reached a conclusion on this. And to be honest, I'm not
> sure what the right way forward is in this situation. So in order to
> make some forward progress I suggest we start a discussion, hopefully
> that will clarify the situation and help lead to the conclusion. Let me
> recap where we are:
>
> Boris' series has two goals: 1) allow seamless hand-off from firmware to
> kernel of a PWM channel and 2) apply changes to a regulator in a single
> atomic operation. To achieve this the concept of PWM state is introduced
> which encapsulates the settings of a PWM channel. On driver probe the
> current state will be read from hardware and when one or more parameters
> are to be changed, the current state is duplicated, the new values set
> in the state and the new state applied.
>
> The problem that we've encountered is that since the PWM parameters are
> specified in DT (or board files), there is the possibility of the PWM
> hardware state and the board parameters disagreeing. To resolve such
> situations there must be a point in time where both hardware state and
> software state must be synchronized.
Maybe we're just hunting ghosts here. There's only one valid state, and
this is the 'hardware state'. I reused the pwm_state struct to store the
reference config (what you call software state), but actually that's
not a state, that's a configuration template, it will only become the
PWM state after being applied.
Maybe I should use another struct to clarify that.
> Now the most straightforward way to
> do that would be to simply apply the software state and be done with it.
Why that? I mean what's the problem of having a reference/template
config that PWM users can rely on if they want to, and still keep
the real state retrieved from hardware read-out? This way PWM users can
decide which one they want to use.
> However the software state initially lacks the duty cycle because it is
> a parameter that usually depends on the use-case (for backlight for
> instance it controls the brightness, for regulators it controls the
> output voltage, ...).
Yes, and I perfectly understand that, we don't want to encode the
default duty cycle in the DT.
>
> Applying the software state as-is also means that there's no reason at
> all to read out the hardware state in the first place, because it will
> simply be discarded.
Yes, but as stated above, this can be a source of glitches, which is
exactly what we're trying to avoid.
>
> An alternative would be to discard the software state and trust the
> hardware to be configured correctly. That's somewhat risky because we
> don't know if the hardware is properly configured. Or Linux might have
> different requirements from the firmware and hence needs to configure
> the PWM differently.
Yep, I agree on that too.
>
> Neither of the above are very attractive options. The best I've been
> able to come up with so far is to completely remove this decision from
> the PWM subsystem and let users handle this. That is, a PWM regulator
> driver would have to have all the knowledge about how to configure the
> PWM for its needs. So upon probe, the PWM regulator driver would inspect
> the current state of the PWM and adjust if necessary, then apply again.
> Ideally of course it wouldn't have to do anything because the hardware
> PWM state would match the software configuration. The idea here is that
> the PWM regulator driver knows exactly what duty cycle to configure to
> obtain the desired output voltage.
That complicates a lot of things. I also suggested another approach:
provide an API to express the duty-cycle value relatively to the period
value (I actually sent a proposal for that, but you didn't comment on
it).
This would keep the PWM user implementations simple, and let the PWM
core switch from the bootloader/firmware period/polarity config to the
DT/PWM-lookup-table one on the first pwm_set_rel_duty_cycle() call.
>
> That doesn't really get us closer, though. There is still the issue of
> the user having to deal with two states: the current hardware state and
> the software state as configured in DT or board files.
At the risk of repeating myself, I don't think we ever had a software
state, and existing PWM users are just assuming that the PWM devices
they are requesting were not used before. IOW, yes, patching PWM users
to support smoother transitions on the PWM devices require some changes,
but in the other hand, that's not like they were doing any better
before that series. And they still have the choice to completely ignore
the current PWM state and blindly apply their own config (extracted
from the reference PWM config)
Best Regards,
Boris
>
> Like I said, I'm on the fence about this, so I'd appreciate any comments
> and perhaps insight from user subsystem maintainers on how they'd like
> this to look, or how this has been done with other resources (GPIOs,
> ...?)
>
> Thierry
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Doug Anderson @ 2016-01-25 18:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160125170855.GA10182@ulmo>
Hi,
On Mon, Jan 25, 2016 at 9:08 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> I really don't understand this design decision. I presume that the PWM
> controlling this system-critical logic is driven by the SoC? So if the
> regulator is system-critical, doesn't that make it a chicken and egg
> problem? How can the SoC turn the PWM on if it doesn't have power? But
> perhaps I'm completely misunderstanding what you're saying. Perhaps if
> somebody could summarize how exactly this works, it would help better
> understand the requirements or what's the correct thing to do.
Sure, here's how the dang thing works, as I understand it.
First, an overview of PWM regulator in general (maybe you know this,
but to get us on the same page). There's an external regulator on the
system. Looking on at least one board I see a TLV62565 specifically.
From the docs of TLV62565, I see it describe the situation as the chip
being able to provide an adjustable output voltage configurable via an
external resistor divider. In simplified terms words you can adjust
the output voltage of the regulator by tweaking the inputs to one of
its pins. I'm just a software guy so I can't explain all the details
of it, but the net-net of the situation is is that you can hook this
configuration pin up to the output of a PWM (with a bunch of well
balanced resistors and capacitors) and then you can set the voltage
based on the output of the PWM.
OK, so what happens at bootup? At bootup most of the pins of the
rk3288 (including the PWM) are configured as inputs with a pull. The
particular pin hooked up to this PWM has a pulldown. Remember that
we've got this nicely balanced set of resistors and capacitors hooked
up to the output of our PWM pin? So what happens when we have this
pin configured as an input? As I understand it / remember it:
* input w/ no pull: equivalent to 50% duty cycle on the PWM
* input w/ pull down: equivalent to slightly higher voltage than 50%
duty cycle on the PWM
* input w/ pull up: equivalent to slightly lower voltage than 50% duty
cycle on the PWM
On our particular board that means that the rail comes up with roughly
1.1V. If you drive the PWM at 100% (or set the pin to output high)
you get .86V and if you drive the PWM at 0% (or set the pin to output
low) you get 1.36V.
Now, 1.1V is plenty of voltage to boot the system. In fact most of
the logic within the SoC can run as low as 0.95V I think. ...but 0.86
V is not enough to run the logic parts of the system (even at their
default bootup frequencies) 1.1V is _definitely_ not enough to run the
SDRAM memory controller at full speed.
So the bootloader wants to run the system fast so it can boot fast.
It increases the CPU rails (as is typical for a bootloader) and moves
the ARM CPU to 1.8GHz (from the relatively slow boot frequency) and
also raises the logic rail to 1.2V (or I think 1.15 V on systems w/
different memory configs) and inits the SDRAM controller to run at
full speed. Then it boots Linux.
Note: apparently in U-Boot they actually boot system slower (this was
at least true 1.5 years ago with some reference U-Boot Rockchip
provided). If I understand correctly they _didn't_ init the SDRAM
controller as full speed in the bootloader and just left the logic
rail at its bootup default. If everyone had done that then our job
would be "easier" because we wouldn't need to read in the voltage
provided by the bootloader (by reading the PWM and cros-referencing
with our table), though even in that case we'd have to be very careful
not to glitch the line (since .86 V is too low). Of course all of
those systems are stuck running at a very slow memory speed until
Linux gets DDR Frequency support for Rockchip whereas systems with our
bootloader not only boot faster but also get to use the full memory
speed even without any Linux DDRFreq drivers.
In any case: I think I've demonstrated how a critical system rail can
be using a PWM regulator and how glitching that PWM regulator at boot
time can be catastrophic. Possibly it's not critical to be able to
"read" the voltage that that bootloader left things configured at
(it's mostly nice for debugging purposes), but it's definitely
important to make sure we don't set it to some default and important
to never glitch it. Said another way, presumably a DDR Freq driver
would be able to switch the memory controller frequency sanely by
reading the memory controller frequency and using that to figure out
whether it needed to up the logic rail before or after the DDR Freq
change.
>> If there's already been off-list discussion and Boris already knows
>> what the next steps are then my apologies and I'll wait patiently for
>> the next series. ;)
>
> I don't think we reached a conclusion on this. And to be honest, I'm not
> sure what the right way forward is in this situation. So in order to
> make some forward progress I suggest we start a discussion, hopefully
> that will clarify the situation and help lead to the conclusion. Let me
> recap where we are:
>
> Boris' series has two goals: 1) allow seamless hand-off from firmware to
> kernel of a PWM channel and 2) apply changes to a regulator in a single
> atomic operation. To achieve this the concept of PWM state is introduced
> which encapsulates the settings of a PWM channel. On driver probe the
> current state will be read from hardware and when one or more parameters
> are to be changed, the current state is duplicated, the new values set
> in the state and the new state applied.
At at even higher level the goal is to support PWM regulator for a
system-critical rail without ever glitching. If we could solve that
problem in some other way that would also be fine too, I think.
> The problem that we've encountered is that since the PWM parameters are
> specified in DT (or board files), there is the possibility of the PWM
> hardware state and the board parameters disagreeing. To resolve such
> situations there must be a point in time where both hardware state and
> software state must be synchronized. Now the most straightforward way to
> do that would be to simply apply the software state and be done with it.
> However the software state initially lacks the duty cycle because it is
> a parameter that usually depends on the use-case (for backlight for
> instance it controls the brightness, for regulators it controls the
> output voltage, ...).
Excuse me for not knowing all details that have been talked about before, but...
A) The software state here is the period and flags (AKA "inverted),
right? It does seem possible that you could apply the period and
flags while keeping the calculated bootup duty cycle percentage
(presuming that the PWM was actually enabled at probe time and there
was a bootup duty cycle at all). That would basically say that
whenever you set the period of a PWM then the duty cycle of the PWM
should remain the same percentage. That actually seems quite sane
IMHO. It seems much saner than trying to keep the duty cycle "ns"
when the period changes or resetting the PWM to some default when the
period changes.
B) Alternatively, I'd also say that setting a period without a duty
cycle doesn't make a lot of sense. ...so you could just apply the
period at the same time that you apply the duty cycle the first time.
Presumably you'd want to "lie" to the callers of the PWM subsystem and
tell them that you already changed the period even though the change
won't really take effect until they actually set the duty cycle. If
anyone cared to find out the true hardware period we could add a new
pwm_get_hw_period(). ...or since the only reason you'd want to know
the hardware period would be if you're trying to read the current duty
cycle percentage, you could instead add "pwm_get_hw_state()" and have
that return both the hardware period ns and duty cycle ns (which is
the most accurate way to return the "percentage" without using fix or
floating point math).
Both of the above options seems like it could be sensible. The 2nd
seems cleaner because it doesn't require you to recalculate /
approximate the old duty cycle using a new period, but it's slightly
uglier because it no longer returns the true hardware state from
pwm_get_period().
> Applying the software state as-is also means that there's no reason at
> all to read out the hardware state in the first place, because it will
> simply be discarded.
Pretty sure we can't discard the hardware duty cycle at bootup, as per above.
> An alternative would be to discard the software state and trust the
> hardware to be configured correctly. That's somewhat risky because we
> don't know if the hardware is properly configured. Or Linux might have
> different requirements from the firmware and hence needs to configure
> the PWM differently.
Doesn't seem like a good idea either.
> Neither of the above are very attractive options. The best I've been
> able to come up with so far is to completely remove this decision from
> the PWM subsystem and let users handle this. That is, a PWM regulator
> driver would have to have all the knowledge about how to configure the
> PWM for its needs. So upon probe, the PWM regulator driver would inspect
> the current state of the PWM and adjust if necessary, then apply again.
> Ideally of course it wouldn't have to do anything because the hardware
> PWM state would match the software configuration. The idea here is that
> the PWM regulator driver knows exactly what duty cycle to configure to
> obtain the desired output voltage.
I think this is like my suggestion B), right? AKA the PWM regulator
would be the sole caller of pwm_get_hw_state() and it would use this
to figure out the existing duty cycle percentage. Then it would
translate that into "ns" and would set the duty cycle. Upon the first
set of the duty cycle both the period and duty cycle would be applied
at the same time.
> That doesn't really get us closer, though. There is still the issue of
> the user having to deal with two states: the current hardware state and
> the software state as configured in DT or board files.
I think the only users that need to deal with this are one that need a
seamless transition from bootup settings. Adding a new API call to
support a new feature like this doesn't seem insane, and anyone who
doesn't want this new feature can just never call the new API.
The only thing that would "change" from the point of view of old
drivers is that the PWM period wouldn't change at bootup until the
duty cycle was set. IMHO this is probably a bug fix. AKA, for a PWM
backlight, imagine:
1. Firmware sets period to 20000 ns, duty cycle to 8000 ns (40%)
2. Linux boots up and sets period to 10000 ns. Brightness of
backlight instantly goes to 80%.
3. Eventually something decides to set the backlight duty cycle and it
goes to the proper rate.
Skipping #2 seems like the right move. ...or did I misunderstand how
something works?
> Like I said, I'm on the fence about this, so I'd appreciate any comments
> and perhaps insight from user subsystem maintainers on how they'd like
> this to look, or how this has been done with other resources (GPIOs,
> ...?)
-Doug
^ permalink raw reply
* Re: [PATCH] fbdev: s6e8ax0: avoid unused function warnings
From: Krzysztof Kozlowski @ 2016-01-26 0:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1453734300-1958304-1-git-send-email-arnd@arndb.de>
On 26.01.2016 00:04, Arnd Bergmann wrote:
> The s6e8ax0 suspend/resume functions are hidden inside of an #ifdef
> when CONFIG_PM is set to avoid unused function warnings, but they
> call some other functions that nothing else calls, and we get warnings
> about those:
>
> drivers/video/fbdev/exynos/s6e8ax0.c:449:13: error: 's6e8ax0_sleep_in' defined but not used [-Werror=unused-function]
> drivers/video/fbdev/exynos/s6e8ax0.c:485:13: error: 's6e8ax0_display_off' defined but not used [-Werror=unused-function]
>
> This marks the PM functions as __maybe_unused so the compiler can
> silently drop them when they are not referenced.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/video/fbdev/exynos/s6e8ax0.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH] video: fbdev: mxsfb: fix pixelclock polarity
From: Stefan Agner @ 2016-01-26 1:29 UTC (permalink / raw)
To: plagnioj, shawnguo
Cc: s.hauer, Ying.Liu, linux-fbdev, linux-kernel, LW, fabio.estevam,
max.krummenacher, Stefan Agner
The PIXDATA flags of the display_flags enum are controller centric,
e.g. NEGEDGE means the controller shall drive the data signals on
pixelclocks negative edge. However, the drivers flag is display
centric: Sample the data on negative (falling) edge.
Therefore, change the if statement to check for the POSEDGE flag
(which is typically not set):
Drive on positive edge => sample on negative edge
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
Hi all
Shawn, I would like to have at least your Ack on this before merge.
It seems that this has been wrong since the driver is able to use
the timings from the device tree, introduced with 669406534b4a
("video: mxsfb: get display timings from device tree").
Not sure how many device trees actually specify the wrong pixel clock
polarity due to that. At least the initial flag convertion from the
old platform data structures done with 0d9f8217db15 ("ARM: mxs: move
display timing configurations into device tree") seems to be affected
and would need to be changed accordingly...
Not sure how we should handle this, maybe just invert all
pixelclk-active properties where the mxsfb driver is in use...?
--
Stefan
drivers/video/fbdev/mxsfb.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
index 4e6608c..38898a9 100644
--- a/drivers/video/fbdev/mxsfb.c
+++ b/drivers/video/fbdev/mxsfb.c
@@ -150,7 +150,7 @@
#define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24 bit width */
#define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT (1 << 6)
-#define MXSFB_SYNC_DOTCLK_FALLING_ACT (1 << 7) /* negtive edge sampling */
+#define MXSFB_SYNC_DOTCLK_FALLING_ACT (1 << 7) /* negative edge sampling */
enum mxsfb_devtype {
MXSFB_V3,
@@ -788,7 +788,16 @@ static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host,
if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
host->sync |= MXSFB_SYNC_DATA_ENABLE_HIGH_ACT;
- if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+
+ /*
+ * The PIXDATA flags of the display_flags enum are controller
+ * centric, e.g. NEGEDGE means drive data on negative edge.
+ * However, the drivers flag is display centric: Sample the
+ * data on negative (falling) edge. Therefore, check for the
+ * POSEDGE flag:
+ * drive on positive edge => sample on negative edge
+ */
+ if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
host->sync |= MXSFB_SYNC_DOTCLK_FALLING_ACT;
put_display_node:
--
2.7.0
^ permalink raw reply related
* [PATCH resend] ocfb: fix tgdel and tvdel timing parameters
From: Andrea Merello @ 2016-01-26 14:48 UTC (permalink / raw)
To: linux-fbdev
According to the ocfb documentation:
Fix tgdel HW param should be left margin, not right.
Fix tvdel HW param should upper margin, not lower.
This seems to fix lock issues on certain monitors (tested on a
slightly customized IP, but the FPGA guy said that it should
be the same wrt this changes).
Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
diff --git a/drivers/video/fbdev/ocfb.c b/drivers/video/fbdev/ocfb.c
index c9293ae..a970edc2 100644
--- a/drivers/video/fbdev/ocfb.c
+++ b/drivers/video/fbdev/ocfb.c
@@ -123,11 +123,11 @@ static int ocfb_setupfb(struct ocfb_dev *fbdev)
/* Horizontal timings */
ocfb_writereg(fbdev, OCFB_HTIM, (var->hsync_len - 1) << 24 |
- (var->right_margin - 1) << 16 | (var->xres - 1));
+ (var->left_margin - 1) << 16 | (var->xres - 1));
/* Vertical timings */
ocfb_writereg(fbdev, OCFB_VTIM, (var->vsync_len - 1) << 24 |
- (var->lower_margin - 1) << 16 | (var->yres - 1));
+ (var->upper_margin - 1) << 16 | (var->yres - 1));
/* Total length of frame */
hlen = var->left_margin + var->right_margin + var->hsync_len +
--
1.9.1
^ permalink raw reply related
* Re: [PATCH resend] ocfb: fix tgdel and tvdel timing parameters
From: Stefan Kristiansson @ 2016-01-26 20:36 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1453819684-32742-1-git-send-email-andrea.merello@gmail.com>
On Tue, Jan 26, 2016 at 03:48:04PM +0100, Andrea Merello wrote:
> According to the ocfb documentation:
> Fix tgdel HW param should be left margin, not right.
> Fix tvdel HW param should upper margin, not lower.
>
> This seems to fix lock issues on certain monitors (tested on a
> slightly customized IP, but the FPGA guy said that it should
> be the same wrt this changes).
>
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
I don't have a hardware setup at the moment to test this,
but you are right about the ocfb documentation and looking at the rtl,
it seems right.
Acked-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
^ permalink raw reply
* Re: Whats missing in my new FB DRM driver in ARC... waiting for console_lock to return
From: Alexey Brodkin @ 2016-01-28 14:20 UTC (permalink / raw)
To: Carlos Palminha
Cc: linux-fbdev@vger.kernel.org, xinliang.liu@linaro.org,
Vineet Gupta, dri-devel@lists.freedesktop.org,
tomi.valkeinen@ti.com, laurent.pinchart@ideasonboard.com,
linux-snps-arc@lists.infradead.org
In-Reply-To: <56A123CD.1060706@synopsys.com>
SGkgQ2FybG9zLA0KDQpPbiBUaHUsIDIwMTYtMDEtMjEgYXQgMTg6MzAgKzAwMDAsIENhcmxvcyBQ
YWxtaW5oYSB3cm90ZToNCj4gaGkuLi4NCj4gDQo+IGkganVzdCBmb3VuZCB0aGF0IGl0cyBibG9j
a2luZyB3YWl0aW5nIGZvciBjb25zb2xlX2xvY2suLi4NCj4gQHZpbmVldCwgYWxleGV5OiBpIHRo
aW5rIHRoYXQgY29uc29sZV9sb2NrIGlzIGFyY2hpdGVjdHVyZSBkZXBlbmRlbnQgcmlnaHQ/IERv
IHlvdSBrbm93IGFueSBpc3N1ZSB3aXRoIGNvbnNvbGVfbG9jaw0KPiBmb3IgQVJDPw0KDQpJJ20g
bm90IHJlYWxseSBzdXJlICJjb25zb2xlX2xvY2siIGhhcyBzb21ldGhpbmcgdG8gZG8gd2l0aCBB
UkMgYXJjaGl0ZWN0dXJlLg0KQXQgbGVhc3QgImdpdCBncmVwIGJjb25zb2xlX2xvY2siIGRvZXNu
J3QgZmluZCBhbnl0aGluZyBpbiAiYXJjaC9hcmMiLg0KDQpTbyBJJ2QgYXNzdW1lIHRoaXMgaXMg
YSBnZW5lcmljIHRoaW5nLg0KDQotQWxleGV5
^ permalink raw reply
* Re: Whats missing in my new FB DRM driver in ARC... waiting for console_lock to return
From: Rob Clark @ 2016-01-28 15:42 UTC (permalink / raw)
To: Alexey Brodkin
Cc: linux-fbdev@vger.kernel.org, Vineet Gupta, Carlos Palminha,
dri-devel@lists.freedesktop.org, tomi.valkeinen@ti.com,
laurent.pinchart@ideasonboard.com,
linux-snps-arc@lists.infradead.org
In-Reply-To: <1453990837.4304.15.camel@synopsys.com>
On Thu, Jan 28, 2016 at 9:20 AM, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> wrote:
> Hi Carlos,
>
> On Thu, 2016-01-21 at 18:30 +0000, Carlos Palminha wrote:
>> hi...
>>
>> i just found that its blocking waiting for console_lock...
>> @vineet, alexey: i think that console_lock is architecture dependent right? Do you know any issue with console_lock
>> for ARC?
>
> I'm not really sure "console_lock" has something to do with ARC architecture.
> At least "git grep bconsole_lock" doesn't find anything in "arch/arc".
>
> So I'd assume this is a generic thing.
>
it is a generic thing.. all arch's are afflicted by console_lock..
BR,
-R
^ permalink raw reply
* Kernel blocked at register_framebuffer because console_block
From: zhonghua @ 2016-01-29 9:05 UTC (permalink / raw)
To: linux-fbdev
Hell all,
I have a booting problem because the drm driver register a new
framebuffer device after created it. Im not sure if the system blocked
or crashed, because there were no message output after its internal call
to console_lock. I tried marked the booting sequence, and found the
problem at: down(&console_sem);
and in its internal line is: sem->count--; Im not sure why? The fb
device is a dpi device, if I register another device, such as hdmi, it
would be OK.
^ permalink raw reply
* Re: [PATCH resend] ocfb: fix tgdel and tvdel timing parameters
From: Tomi Valkeinen @ 2016-01-29 11:35 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1453819684-32742-1-git-send-email-andrea.merello@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1296 bytes --]
On 26/01/16 16:48, Andrea Merello wrote:
> According to the ocfb documentation:
> Fix tgdel HW param should be left margin, not right.
> Fix tvdel HW param should upper margin, not lower.
>
> This seems to fix lock issues on certain monitors (tested on a
> slightly customized IP, but the FPGA guy said that it should
> be the same wrt this changes).
>
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>
> diff --git a/drivers/video/fbdev/ocfb.c b/drivers/video/fbdev/ocfb.c
> index c9293ae..a970edc2 100644
> --- a/drivers/video/fbdev/ocfb.c
> +++ b/drivers/video/fbdev/ocfb.c
> @@ -123,11 +123,11 @@ static int ocfb_setupfb(struct ocfb_dev *fbdev)
>
> /* Horizontal timings */
> ocfb_writereg(fbdev, OCFB_HTIM, (var->hsync_len - 1) << 24 |
> - (var->right_margin - 1) << 16 | (var->xres - 1));
> + (var->left_margin - 1) << 16 | (var->xres - 1));
>
> /* Vertical timings */
> ocfb_writereg(fbdev, OCFB_VTIM, (var->vsync_len - 1) << 24 |
> - (var->lower_margin - 1) << 16 | (var->yres - 1));
> + (var->upper_margin - 1) << 16 | (var->yres - 1));
>
> /* Total length of frame */
> hlen = var->left_margin + var->right_margin + var->hsync_len +
> --
> 1.9.1
>
Thanks, queued for 4.5 fixes.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] fbdev: s6e8ax0: avoid unused function warnings
From: Tomi Valkeinen @ 2016-01-29 11:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1453734300-1958304-1-git-send-email-arnd@arndb.de>
[-- Attachment #1: Type: text/plain, Size: 864 bytes --]
On 25/01/16 17:04, Arnd Bergmann wrote:
> The s6e8ax0 suspend/resume functions are hidden inside of an #ifdef
> when CONFIG_PM is set to avoid unused function warnings, but they
> call some other functions that nothing else calls, and we get warnings
> about those:
>
> drivers/video/fbdev/exynos/s6e8ax0.c:449:13: error: 's6e8ax0_sleep_in' defined but not used [-Werror=unused-function]
> drivers/video/fbdev/exynos/s6e8ax0.c:485:13: error: 's6e8ax0_display_off' defined but not used [-Werror=unused-function]
>
> This marks the PM functions as __maybe_unused so the compiler can
> silently drop them when they are not referenced.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/video/fbdev/exynos/s6e8ax0.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
Thanks, queued for 4.5 fixes.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] fbdev: da8xx-fb: remove incorrect type cast
From: Tomi Valkeinen @ 2016-01-29 11:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1453734351-1958373-1-git-send-email-arnd@arndb.de>
[-- Attachment #1: Type: text/plain, Size: 694 bytes --]
On 25/01/16 17:05, Arnd Bergmann wrote:
> The probe function correct passes a dma_addr_t pointer into
> dma_alloc_coherent(), but has a cast to resource_size_t, which
> might be different from dma_addr_t:
>
> drivers/video/fbdev/da8xx-fb.c: In function 'fb_probe':
> drivers/video/fbdev/da8xx-fb.c:1431:10: error: passing argument 3 of 'dma_alloc_coherent' from incompatible pointer type [-Werror=incompatible-pointer-types]
>
> This removes the cast, which avoids the warning.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/video/fbdev/da8xx-fb.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Thanks, queued for 4.5 fixes.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] fbdev: mmp: print IRQ resource using %pR format string
From: Tomi Valkeinen @ 2016-01-29 11:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1453734191-1958245-1-git-send-email-arnd@arndb.de>
[-- Attachment #1: Type: text/plain, Size: 889 bytes --]
On 25/01/16 17:03, Arnd Bergmann wrote:
> resource_size_t cannot be printed using the %x format string
> when we it is defined as u64:
>
> drivers/video/fbdev/mmp/hw/mmp_ctrl.c: In function 'mmphw_probe':
> drivers/video/fbdev/mmp/hw/mmp_ctrl.c:506:22: error: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'resource_size_t {aka long long unsigned int}' [-Werror=format=]
> dev_err(ctrl->dev, "%s: res %x - %x map failed\n", __func__,
> ^
> drivers/video/fbdev/mmp/hw/mmp_ctrl.c:506:22: error: format '%x' expects argument of type 'unsigned int', but argument 5 has type 'resource_size_t {aka long long unsigned int}' [-Werror=format=]
>
> This changes the format string to %pR, which is interpreted
> by the printk implementation to pretty-print a resource
> structure.
Thanks, queued for 4.5 fixes.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] fbdev: ssd1307fb: Fix chargepump setting
From: Tomi Valkeinen @ 2016-01-29 11:55 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1447337257-31232-1-git-send-email-julian@jusst.de>
[-- Attachment #1: Type: text/plain, Size: 1343 bytes --]
On 18/01/16 22:20, Julian Scheel wrote:
> On 15.11.15 14:36, Julian Scheel wrote:
>> The charge pump setting must have bit D4 set all time according to the
>> SSD1306
>> App Note. Instead of doing an logical and off shifted setting bit with
>> 0x14 it
>> must be an logical or with 0x10 to ensure D4 is set.
The code change looks ok, but I can't quite decipher the commit
description. "an logical and off shifted setting bit with 0x14"?
>> Signed-off-by: Julian Scheel <julian@jusst.de>
>> ---
>> drivers/video/fbdev/ssd1307fb.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/ssd1307fb.c
>> b/drivers/video/fbdev/ssd1307fb.c
>> index 1611215..5965a9b 100644
>> --- a/drivers/video/fbdev/ssd1307fb.c
>> +++ b/drivers/video/fbdev/ssd1307fb.c
>> @@ -389,7 +389,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
>> return ret;
>>
>> ret = ssd1307fb_write_cmd(par->client,
>> - (par->device_info->need_chargepump & 0x1 << 2) & 0x14);
>> + 0x10 | ((par->device_info->need_chargepump & 0x01) << 2));
I presume 'need_chargepump' is really supposed to be a bool, instead of
int. And if it's really bool, something like this makes it more readable
to me:
BIT(4) | (par->device_info->need_chargepump ? BIT(2) : 0)
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Yours Truely Mrs Machiko Kumiko
From: Mrs Machiko Kumiko @ 2016-01-29 11:59 UTC (permalink / raw)
To: mrsmachiko_kumiko
Hi Dear,
I am Mrs Machiko Kumiko, from Japan, I Have Been Diagnosed with esophageal Cancer.
I Have Chosen you to distribute my Funds to Charities homes in your Country,
so if you wish to Carry out this humanitarian Work kindly Get back to me for FURTHER details.
Yours Truely Mrs Machiko Kumiko
^ permalink raw reply
* Re: [PATCH] fbdev: ssd1307fb: Fix chargepump setting
From: Julian Scheel @ 2016-01-29 12:00 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1447337257-31232-1-git-send-email-julian@jusst.de>
Am 29.01.2016 um 12:55 schrieb Tomi Valkeinen:
>
> On 18/01/16 22:20, Julian Scheel wrote:
>> On 15.11.15 14:36, Julian Scheel wrote:
>>> The charge pump setting must have bit D4 set all time according to the
>>> SSD1306
>>> App Note. Instead of doing an logical and off shifted setting bit with
>>> 0x14 it
>>> must be an logical or with 0x10 to ensure D4 is set.
>
> The code change looks ok, but I can't quite decipher the commit
> description. "an logical and off shifted setting bit with 0x14"?
Appears I was not awake when writing the commit message. Besides that
the sentence is really cludgy it must be "of" instead of "off".
It might be simplified to: "Applying logical and to 0x14 and the
chargepump bit prevents bit 4 from being set. Fix it by or'ing bit 4 and
the chargepump enable bit"
>>> Signed-off-by: Julian Scheel <julian@jusst.de>
>>> ---
>>> drivers/video/fbdev/ssd1307fb.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/video/fbdev/ssd1307fb.c
>>> b/drivers/video/fbdev/ssd1307fb.c
>>> index 1611215..5965a9b 100644
>>> --- a/drivers/video/fbdev/ssd1307fb.c
>>> +++ b/drivers/video/fbdev/ssd1307fb.c
>>> @@ -389,7 +389,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
>>> return ret;
>>>
>>> ret = ssd1307fb_write_cmd(par->client,
>>> - (par->device_info->need_chargepump & 0x1 << 2) & 0x14);
>>> + 0x10 | ((par->device_info->need_chargepump & 0x01) << 2));
>
> I presume 'need_chargepump' is really supposed to be a bool, instead of
> int. And if it's really bool, something like this makes it more readable
> to me:
>
> BIT(4) | (par->device_info->need_chargepump ? BIT(2) : 0)
Yes, this is easier to read.
Let me update the patch and send a v2.
-Julian
^ permalink raw reply
* Re: [PATCH] video: fbdev: imxfb: Provide a reset mechanism
From: Tomi Valkeinen @ 2016-01-29 12:20 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1453209057-16444-1-git-send-email-festevam@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1302 bytes --]
On 19/01/16 15:10, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@nxp.com>
>
> Currently when we boot the kernel on a mx25pdk the LCDC controller
> does not show the Linux logo on boot.
>
> This problem is well explained by Sascha Hauer:
>
> "Unfortunately this LCD controller does not have an enable bit. The
> controller starts directly when the clocks are enabled. If the clocks
> are enabled when the controller is not yet programmed with proper
> register values then it just goes into some undefined state. What I
> suspect is that the clocks already were enabled before driver probe,
> presumably by the bootloader, so the controller is already in undefined
> state when entering Linux. Now by dis/enabling the ipg clock you
> effectively reset the controller. Since you have programmed it with
> valid register values in the mean time it starts working after this
> reset."
>
> So do as suggested and force a reset of the LCDC hardware by
> enabling and disabling the IPG clock.
>
> With this change the Linux logo can be seen on boot on a mx25pdk.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> drivers/video/fbdev/imxfb.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
Thanks. Queued for 4.5 fixes.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] video: Use bool instead int pointer for get_opt_bool() argument
From: Daniel Wagner @ 2016-01-29 13:23 UTC (permalink / raw)
To: linux-kernel, linux-fbdev; +Cc: Maik Broemme, Daniel Wagner
In-Reply-To: <1453992270-4688-1-git-send-email-daniel.wagner@bmw-carit.de>
As the function name already indicates that get_opt_bool() parses
for a bool. It is not a surprise that compiler is complaining
about it when -Werror=incompatible-pointer-types is used:
drivers/video/fbdev/intelfb/intelfbdrv.c: In function ‘intelfb_setup’:
drivers/video/fbdev/intelfb/intelfbdrv.c:353:39: error: passing argument 3 of ‘get_opt_bool’ from incompatible pointer type [-Werror=incompatible-pointer-types]
if (get_opt_bool(this_opt, "accel", &accel))
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
---
Hi,
In the 'simple wait queue support' series is a patch
which turns on -Werror=incompatible-pointer-types which will
result in a compile error for intelfb.
https://lkml.org/lkml/2016/1/28/462
Even if that patch wont make it, this one makes sense (at least
for me :))
I'll prepend this patch to the next version of the series in order
to see if I got rid of all incompatible pointer types errors caught
by the kbuild test robot.
cheers,
daniel
drivers/video/fbdev/intelfb/intelfbdrv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/intelfb/intelfbdrv.c b/drivers/video/fbdev/intelfb/intelfbdrv.c
index bbec737..bf20744 100644
--- a/drivers/video/fbdev/intelfb/intelfbdrv.c
+++ b/drivers/video/fbdev/intelfb/intelfbdrv.c
@@ -302,7 +302,7 @@ static __inline__ int get_opt_int(const char *this_opt, const char *name,
}
static __inline__ int get_opt_bool(const char *this_opt, const char *name,
- int *ret)
+ bool *ret)
{
if (!ret)
return 0;
--
2.5.0
^ permalink raw reply related
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