* RE: [PATCH v3 00/10] video: da8xx-fb: runtime timing configuration
From: Mohammed, Afzal @ 2013-02-07 9:05 UTC (permalink / raw)
To: Florian Tobias Schandinat, Valkeinen, Tomi, Nori, Sekhar,
linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <cover.1358250489.git.afzal@ti.com>
SGkgVG9taSwgRmxvcmlhbiwNCg0KT24gVHVlLCBKYW4gMTUsIDIwMTMgYXQgMTk6MDA6NTAsIE1v
aGFtbWVkLCBBZnphbCB3cm90ZToNCg0KPiBUaGlzIHNlcmllcyBtYWtlcyBkYTh4eC1mYiBkcml2
ZXIgKGRldmljZSBmb3VuZCBvbiBEYVZpbmNpIGFuZCBBTTMzNXgpDQo+IGNhcGFibGUgb2YgaGFu
ZGxpbmcgcnVudGltZSB0aW1pbmcgY29uZmlndXJhdGlvbiBieSBhZGRpbmcgZmJfc2V0X3Bhci4N
Cj4gDQo+IFRoZSBsYXN0IGNoYW5nZSBhZGRzIGFjdHVhbCBmYl9zZXRfcGFyIHN1cHBvcnQuIE90
aGVyIHByZWNlZWRpbmcNCj4gY2hhbmdlcyBtYWtlcyB0aGUgd2F5IGNsZWFyIGZvciBpdCBhcyB3
ZWxsIGFzIGRvZXMgY2VydGFpbiBjbGVhbnVwJ3MNCj4gb24gdGhlIHdheS4NCj4gDQo+IFRoaXMg
aGFzIGJlZW4gdGVzdGVkIG9uIGRhODUwIGV2bSBhcyBpcy4gVGhpcyB3YXMgYWxzbyB0ZXN0ZWQg
b24NCj4gYW0zMzV4LWV2bSAmIGFtMzM1eC1ldm1zayB3aXRoIGEgc2VyaWVzIHRoYXQgYWRkcyBE
VCBzdXBwb3J0Lg0KPiANCj4gVGhpcyBpcyBiYXNlZCBvbiB2My44LXJjMywgdGhpcyBpcyB0aGUg
b25seSBjaGFuZ2UgaW4gdGhpcyByZXZpc2lvbi4NCj4gVGhlIG5ldyB2ZXJzaW9uIG9mIHRoaXMg
c2VyaWVzIGlzIGJlaW5nIHBvc3RlZCBzbyB0aGF0IHRoaXMgc2VyaWVzIGNhbg0KPiBiZSBhcHBs
aWVkIGVhc2lseSAoYXMgX19kZXYqIGFyZSByZW1vdmVkLCB0aGVyZSB3b3VsZCBiZSBtZXJnZQ0K
PiBjb25mbGljdHMgd2l0aCB2Miwgd2hpY2ggd2FzIGJhc2VkIG9uIC1yYzIpLg0KPiBzZXJpZXMN
Cg0KQ2FuIHlvdSBwbGVhc2UgY29uc2lkZXIgdGhpcyBzZXJpZXMgZm9yIGluY2x1c2lvbi4gVGhl
cmUgYXJlIG5vDQpwZW5kaW5nIGNvbW1lbnRzIG9yIGRlcGVuZGVuY3kgZm9yIHRoaXMgc2VyaWVz
LiBJZiB5b3UgbmVlZCBhDQpwdWxsIHJlcXVlc3QsIGxldCBtZSBrbm93LCBJIHdpbGwgc2VudCBp
dC4NCg0KUmVnYXJkcw0KQWZ6YWwNCg0K
^ permalink raw reply
* Re: Unmerged patches for 3.9
From: Sachin Kamat @ 2013-02-07 7:19 UTC (permalink / raw)
To: linux-fbdev
Resending due to wrong fbdev mailing address. Sorry.
On 7 February 2013 12:34, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> Hi Florian,
>
> I have the following 'Acked' patches missing in your tree.
>
> https://patchwork.kernel.org/patch/1864681/
> https://patchwork.kernel.org/patch/1923041/
> https://patchwork.kernel.org/patch/1926501/
>
> Could you please pick them up in your tree as they have been pending
> almost since a couple of months now.
>
> --
> With warm regards,
> Sachin
--
With warm regards,
Sachin
^ permalink raw reply
* Re: [Linaro-mm-sig] CDF meeting @FOSDEM report
From: Daniel Vetter @ 2013-02-06 16:14 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Alex Deucher, Thomas Petazzoni, linux-fbdev, Stephen Warren,
Thierry Reding, Mark Zhang, dri-devel, Sunil Joshi, linaro-mm-sig,
Stéphane Marchesin, Kyungmin Park, Jesse Barnes,
Laurent Pinchart, Sebastien Guiriec, Alexandre Courbot,
Maxime Ripard, Vikas Sajjan, Ragesh Radhakrishnan, linux-media
In-Reply-To: <51127008.7050808@ti.com>
On Wed, Feb 6, 2013 at 4:00 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> not always a perfect match to the hardware. For example a lot of GPUs
>> have a DVO encoder which feeds a secondary encoder like an sil164 DVO
>> to TMDS encoder.
>
> Right. I think mapping the DRM entities to CDF ones is one of the bigger
> question marks we have with CDF. While I'm no expert on DRM, I think we
> have the following options:
>
> 1. Force DRM's model to CDF, meaning one encoder.
>
> 2. Extend DRM to support multiple encoders in a chain.
>
> 3. Support multiple encoders in a chain in CDF, but somehow map them to
> a single encoder in DRM side.
4. Ignore drm kms encoders.
They are only exposed to userspace as a means for userspace to
discover very simple constraints, e.g. 1 encoder connected to 2
outputs means you can only use one of the outputs at the same time.
They are completely irrelevant for the actual modeset interface
exposed to drivers, so you could create a fake kms encoder for each
connector you expose through kms.
The crtc helpers use the encoders as a real entity, and if you opt to
use the crtc helpers to implement the modeset sequence in your driver
it makes sense to map them to some real piece of hw. But you can
essentially pick any transcoder in your crtc -> final output chain for
this. Generic userspace needs to be able to cope with a failed modeset
due to arbitrary reasons anyway, so can't presume that simply because
the currently exposed constraints are fulfilled it'll work.
> I really dislike the first option, as it would severely limit where CDF
> can be used, or would force you to write some kind of combined drivers,
> so that you can have one encoder driver running multiple encoder devices.
Imo CDF and drm encoders don't really have that much to do with each
another, it should just be a driver implementation detail. Of course,
if common patterns emerge we could extract them somehow. E.g. if many
drivers end up exposing the CDF transcoder chain as a drm encoder
using the crtc helpers, we could add some library functions to make
that simpler.
Another conclusion (at least from my pov) from the fosdem discussion
is that we should separate the panel interface from the actual
control/pixel data buses. That should give us more flexibility for
insane hw and also directly exposing properties and knobs to the
userspace interface from e.g. dsi transcoders. So I don't think we'll
end up with _the_ canonical CDF sink interface anyway.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: CDF meeting @FOSDEM report
From: Tomi Valkeinen @ 2013-02-06 15:00 UTC (permalink / raw)
To: Alex Deucher
Cc: Laurent Pinchart, linux-fbdev, Sebastien Guiriec, dri-devel,
Jesse Barnes, Benjamin Gaignard, Sumit Semwal, Tom Gall,
Kyungmin Park, linux-media, Stephen Warren, Thierry Reding,
Mark Zhang, linaro-mm-sig, Stéphane Marchesin,
Alexandre Courbot, Ragesh Radhakrishnan, Thomas Petazzoni,
Sunil Joshi, Maxime Ripard, Vikas Sajjan
In-Reply-To: <CADnq5_P1GFbAwoe9kTeARq8ZLP1tOBc9Rn1h2KrRYxkoLxLXfw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1840 bytes --]
On 2013-02-06 16:44, Alex Deucher wrote:
> On Wed, Feb 6, 2013 at 6:11 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> What is an encoder? Something that takes a video signal in, and lets the
>> CPU store the received data to memory? Isn't that a decoder?
>>
>> Or do you mean something that takes a video signal in, and outputs a
>> video signal in another format? (transcoder?)
>
> In KMS parlance, we have two objects a crtc and an encoder. A crtc
> reads data from memory and produces a data stream with display timing.
> The encoder then takes that datastream and timing from the crtc and
> converts it some sort of physical signal (LVDS, TMDS, DP, etc.). It's
Isn't the video stream between CRTC and encoder just as physical, it
just happens to be inside the GPU?
This is the case for OMAP, at least, where DISPC could be considered
CRTC, and DSI/HDMI/etc could be considered encoder. The stream between
DISPC and DSI/HDMI is plain parallel RGB signal. The video stream could
as well be outside OMAP.
> not always a perfect match to the hardware. For example a lot of GPUs
> have a DVO encoder which feeds a secondary encoder like an sil164 DVO
> to TMDS encoder.
Right. I think mapping the DRM entities to CDF ones is one of the bigger
question marks we have with CDF. While I'm no expert on DRM, I think we
have the following options:
1. Force DRM's model to CDF, meaning one encoder.
2. Extend DRM to support multiple encoders in a chain.
3. Support multiple encoders in a chain in CDF, but somehow map them to
a single encoder in DRM side.
I really dislike the first option, as it would severely limit where CDF
can be used, or would force you to write some kind of combined drivers,
so that you can have one encoder driver running multiple encoder devices.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* Re: CDF meeting @FOSDEM report
From: Alex Deucher @ 2013-02-06 14:44 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Thomas Petazzoni, linux-fbdev, Tom Gall, Stephen Warren,
Thierry Reding, Mark Zhang, dri-devel, Sunil Joshi, linaro-mm-sig,
Stéphane Marchesin, Kyungmin Park, Jesse Barnes,
Laurent Pinchart, Sebastien Guiriec, Alexandre Courbot,
Maxime Ripard, Vikas Sajjan, Sumit Semwal, Ragesh Radhakrishnan,
linux-media
In-Reply-To: <51123A5F.9050604@ti.com>
On Wed, Feb 6, 2013 at 6:11 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Hi,
>
> On 2013-02-06 00:27, Laurent Pinchart wrote:
>> Hello,
>>
>> We've hosted a CDF meeting at the FOSDEM on Sunday morning. Here's a summary
>> of the discussions.
>
> Thanks for the summary. I've been on a longish leave, and just got back,
> so I haven't read the recent CDF discussions on lists yet. I thought
> I'll start by replying to this summary first =).
>
>> 0. Abbreviations
>> ----------------
>>
>> DBI - Display Bus Interface, a parallel video control and data bus that
>> transmits data using parallel data, read/write, chip select and address
>> signals, similarly to 8051-style microcontroller parallel busses. This is a
>> mixed video control and data bus.
>>
>> DPI - Display Pixel Interface, a parallel video data bus that transmits data
>> using parallel data, h/v sync and clock signals. This is a video data bus
>> only.
>>
>> DSI - Display Serial Interface, a serial video control and data bus that
>> transmits data using one or more differential serial lines. This is a mixed
>> video control and data bus.
>
> In case you'll re-use these abbrevs in later posts, I think it would be
> good to mention that DPI is a one-way bus, whereas DBI and DSI are
> two-way (perhaps that's implicit with control bus, though).
>
>> 1. Goals
>> --------
>>
>> The meeting started with a brief discussion about the CDF goals.
>>
>> Tomi Valkeinin and Tomasz Figa have sent RFC patches to show their views of
>> what CDF could/should be. Many others have provided very valuable feedback.
>> Given the early development stage propositions were sometimes contradictory,
>> and focused on different areas of interest. We have thus started the meeting
>> with a discussion about what CDF should try to achieve, and what it shouldn't.
>>
>> CDF has two main purposes. The original goal was to support display panels in
>> a platform- and subsystem-independent way. While mostly useful for embedded
>> systems, the emergence of platforms such as Intel Medfield and ARM-based PCs
>> that blends the embedded and PC worlds makes panel support useful for the PC
>> world as well.
>>
>> The second purpose is to provide a cross-subsystem interface to support video
>> encoders. The idea originally came from a generalisation of the original RFC
>> that supported panels only. While encoder support is considered as lower
>> priority than display panel support by developers focussed on display
>> controller driver (Intel, Renesas, ST Ericsson, TI), companies that produce
>> video encoders (Analog Devices, and likely others) don't share that point of
>> view and would like to provide a single encoder driver that can be used in
>> both KMS and V4L2 drivers.
>
> What is an encoder? Something that takes a video signal in, and lets the
> CPU store the received data to memory? Isn't that a decoder?
>
> Or do you mean something that takes a video signal in, and outputs a
> video signal in another format? (transcoder?)
In KMS parlance, we have two objects a crtc and an encoder. A crtc
reads data from memory and produces a data stream with display timing.
The encoder then takes that datastream and timing from the crtc and
converts it some sort of physical signal (LVDS, TMDS, DP, etc.). It's
not always a perfect match to the hardware. For example a lot of GPUs
have a DVO encoder which feeds a secondary encoder like an sil164 DVO
to TMDS encoder.
Alex
^ permalink raw reply
* Re: [RFC v2 0/5] Common Display Framework
From: Archit Taneja @ 2013-02-06 9:52 UTC (permalink / raw)
To: Marcus Lorentzon
Cc: Laurent Pinchart, Thomas Petazzoni, linux-fbdev@vger.kernel.org,
Vikas Sajjan, Benjamin Gaignard, Tom Gall, Kyungmin Park,
dri-devel@lists.freedesktop.org, Rob Clark, Ragesh Radhakrishnan,
Tomi Valkeinen, Bryan Wu, Maxime Ripard, sunil joshi,
Sumit Semwal, Sebastien Guiriec, linux-media@vger.kernel.org
In-Reply-To: <510F8807.2020406@stericsson.com>
On Monday 04 February 2013 03:35 PM, Marcus Lorentzon wrote:
> On 02/02/2013 12:35 AM, Laurent Pinchart wrote:
>> Hi Marcus,
>>
>> On Tuesday 08 January 2013 18:08:19 Marcus Lorentzon wrote:
>>> On 01/08/2013 05:36 PM, Tomasz Figa wrote:
>>>> On Tuesday 08 of January 2013 11:12:26 Marcus Lorentzon wrote:
> [...]
>>>>> But it is not perfect. After a couple of products we realized that
>>>>> most
>>>>> panel drivers want an easy way to send a bunch of init commands in one
>>>>> go. So I think it should be an op for sending an array of commands at
>>>>> once. Something like
>>>>>
>>>>> struct dsi_cmd {
>>>>> enum mipi_pkt_type type; /* MIPI DSI, DCS, SetPacketLen, ... */
>>>>> u8 cmd;
>>>>> int dataLen;
>>>>> u8 *data;
>>>>> }
>>>>>
>>>>> struct dsi_ops {
>>>>> int dsi_write(source, int num_cmds, struct dsi_cmd *cmds);
>>>>> ...
>>>>> }
>> Do you have DSI IP(s) that can handle a list of commands ? Or would
>> all DSI
>> transmitter drivers need to iterate over the commands manually ? In
>> the later
>> case a lower-level API might be easier to implement in DSI transmitter
>> drivers. Helper functions could provide the higher-level API you
>> proposed.
>
> The HW has a FIFO, so it can handle a few. Currently we use the low
> level type of call with one call per command. But we have found DSI
> command mode panels that don't accept any commands during the "update"
> (write start+continues). And so we must use a mutex/state machine to
> exclude any async calls to send DSI commands during update. But if you
> need to send more than one command per frame this will be hard (like
> CABC and backlight commands). It will be a ping pong between update and
> command calls. One option is to expose the mutex to the caller so it can
> make many calls before the next update grabs the mutex again.
> So maybe we could create a helper that handle the op for list of
> commands and another op for single command that you actually have to
> implement.
fyi, the DSI IP on OMAP3+ SoCs also has a FIFO. It can provide
interrupts after each command is pushed out, and also when the FIFO gets
empty(all commands are pushed). The only thing to take care is to not
overflow FIFO.
DSI video mode panels generally have a few dozen internal registers
which need to be configured via DSI commands. It's more fast(and
convenient) to configure a handful of internal registers in one shot,
and then perform a single BTA to know from the panel whether the
commands were received correctly.
Regards,
Archit
^ permalink raw reply
* Re: 答复: [PATCH]Siliconmotion initial patch
From: Greg KH @ 2013-02-05 21:19 UTC (permalink / raw)
To: Daniel Vetter
Cc: Linux Fbdev development list, caesar.qiu 裘赛海,
dri-devel, Aaron.Chen 陈俊杰
In-Reply-To: <20130205210427.GF5813@phenom.ffwll.local>
On Tue, Feb 05, 2013 at 10:04:27PM +0100, Daniel Vetter wrote:
> On Tue, Feb 05, 2013 at 03:51:55PM +0800, Aaron.Chen 陈俊杰 wrote:
> > This is an initial patch for Siliconmotion Graphics chips. It add SM750
> > chip support with 2d accelerate and hw curser support. It is a complete
> > new driver. So the patch is a little bit big. Is it OK for review?
>
> Adding a new fbdev driver while established consensus pretty much boils
> down to fbdev being a dead subsystem which should only be used for legacy
> applications and drivers: Not good. You should implement this as a drm
> modesetting driver, and if required base your 2d acceleration on top of
> the drm fb helpers. Or just implement a drm/gem driver to expose this.
>
> Additionally you should include the appropriate mailing lists, which is
> linux-fbdev for fbdev drivers.
>
> On a very quick read the patch has serious issues:
>
> - checkpatch.pl:
>
> total: 2 errors, 779 warnings, 7961 lines checked
>
> NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
> scripts/cleanfile
>
> - Remants of hungarian notation (dunno whether checkpatch checks for
> those, too much other stuff flying by, but it really stuck out).
>
> - Not using the linux i2c layer in the ddk750_hwi2c.c file.
>
> - Reimplementing i2c bit-banging code in the ddk750_swi2c.c
>
> - Adding a new implementation of a sil 164 dvi driver, we already have at
> least two in drivers/gpu/drm/i2c/sil164_drv.c and
> drivers/gpu/drm/i915/dvo_sil164.c. This should be unified and probably
> use the drm encoder slave framework.
>
> At that point I've given up on further review, since there's clearly a lot
> of work left to do. Please review SubmittingPatches from the kernel
> documentation carefully. Also cc'ing our dear staging maintainer, he
> should be able to point you at further useful resources for getting this
> going.
If you wish to put this in the drivers/staging/ directory and do the
cleanup there before it moves to the "real" part of the kernel, I have
no objection to that.
thanks,
greg k-h
^ permalink raw reply
* Re: 答复: [PATCH]Siliconmotion initial patch
From: Daniel Vetter @ 2013-02-05 21:04 UTC (permalink / raw)
To: Aaron.Chen 陈俊杰
Cc: Greg KH, Linux Fbdev development list,
caesar.qiu 裘赛海, dri-devel
In-Reply-To: <4CE6A5494DEECD498EBCD4C5488B1AFCEAC6F7@CNDC08.cn.smi.ad>
On Tue, Feb 05, 2013 at 03:51:55PM +0800, Aaron.Chen 陈俊杰 wrote:
> This is an initial patch for Siliconmotion Graphics chips. It add SM750
> chip support with 2d accelerate and hw curser support. It is a complete
> new driver. So the patch is a little bit big. Is it OK for review?
Adding a new fbdev driver while established consensus pretty much boils
down to fbdev being a dead subsystem which should only be used for legacy
applications and drivers: Not good. You should implement this as a drm
modesetting driver, and if required base your 2d acceleration on top of
the drm fb helpers. Or just implement a drm/gem driver to expose this.
Additionally you should include the appropriate mailing lists, which is
linux-fbdev for fbdev drivers.
On a very quick read the patch has serious issues:
- checkpatch.pl:
total: 2 errors, 779 warnings, 7961 lines checked
NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
scripts/cleanfile
- Remants of hungarian notation (dunno whether checkpatch checks for
those, too much other stuff flying by, but it really stuck out).
- Not using the linux i2c layer in the ddk750_hwi2c.c file.
- Reimplementing i2c bit-banging code in the ddk750_swi2c.c
- Adding a new implementation of a sil 164 dvi driver, we already have at
least two in drivers/gpu/drm/i2c/sil164_drv.c and
drivers/gpu/drm/i915/dvo_sil164.c. This should be unified and probably
use the drm encoder slave framework.
At that point I've given up on further review, since there's clearly a lot
of work left to do. Please review SubmittingPatches from the kernel
documentation carefully. Also cc'ing our dear staging maintainer, he
should be able to point you at further useful resources for getting this
going.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH 5/5] ARM: at91/avr32/atmel_lcdfb: replace cpu_is macros with device-id table
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-02-05 20:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1360071315-4032-6-git-send-email-jhovold@gmail.com>
On 14:35 Tue 05 Feb , Johan Hovold wrote:
> Remove cpu_is macros from atmel lcdfb driver and use platform-device-id
> table to determine platform configuration parameters.
>
> The currently used configuration parameters are:
>
> have_alt_pixclock
> - SOC uses an alternate pixel-clock calculation formula (at91sam9g45
> non-ES)
>
> have_bus_clk
> - SOC has bus clock hck1 (at91sam9261, at921sam9g10 and at32ap)
no provide a clkdev a we do for macb
>
> have_hozval
> - SOC has a HOZVAL field in LCDFRMCFG which is used to determine the
> linesize for STN displays (at91sam9261, at921sam9g10 and at32ap)
>
> have_intensity_bit
> - SOC uses IBGR:555 rather than BGR:565 16-bit pixel layout
> (at91sam9261, at91sam9263 and at91sam9rl)
>
> Tested on at91sam9g45, compile-tested for other AT91 SOCs, and untested
> for AVR32.
>
> Signed-off-by: Johan Hovold <jhovold@gmail.com>
> ---
> arch/arm/mach-at91/at91sam9261_devices.c | 6 +-
> arch/arm/mach-at91/at91sam9263_devices.c | 2 +-
> arch/arm/mach-at91/at91sam9g45_devices.c | 6 +-
> arch/arm/mach-at91/at91sam9rl_devices.c | 2 +-
> arch/avr32/mach-at32ap/at32ap700x.c | 2 +
> drivers/video/atmel_lcdfb.c | 96 ++++++++++++++++++++++++++++----
> include/video/atmel_lcdc.h | 4 +-
> 7 files changed, 101 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/mach-at91/at91sam9261_devices.c b/arch/arm/mach-at91/at91sam9261_devices.c
> index 92e0f86..01647cb 100644
> --- a/arch/arm/mach-at91/at91sam9261_devices.c
> +++ b/arch/arm/mach-at91/at91sam9261_devices.c
> @@ -488,7 +488,6 @@ static struct resource lcdc_resources[] = {
> };
>
> static struct platform_device at91_lcdc_device = {
> - .name = "atmel_lcdfb",
> .id = 0,
> .dev = {
> .dma_mask = &lcdc_dmamask,
> @@ -505,6 +504,11 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data)
> return;
> }
>
> + if (cpu_is_at91sam9g10())
> + at91_lcdc_device.name = "fb-at91sam9g10";
use this
at91sam9g10-lcdfb
as we will use for dt
> + else
> + at91_lcdc_device.name = "fb-at91sam9261";
> +
> #if defined(CONFIG_FB_ATMEL_STN)
> at91_set_A_periph(AT91_PIN_PB0, 0); /* LCDVSYNC */
> at91_set_A_periph(AT91_PIN_PB1, 0); /* LCDHSYNC */
> diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
> index ed666f5..a34f39a 100644
> --- a/arch/arm/mach-at91/at91sam9263_devices.c
> +++ b/arch/arm/mach-at91/at91sam9263_devices.c
> @@ -848,7 +848,7 @@ static struct resource lcdc_resources[] = {
> };
>
> static struct platform_device at91_lcdc_device = {
> - .name = "atmel_lcdfb",
> + .name = "fb-at91sam9263",
> .id = 0,
> .dev = {
> .dma_mask = &lcdc_dmamask,
> diff --git a/arch/arm/mach-at91/at91sam9g45_devices.c b/arch/arm/mach-at91/at91sam9g45_devices.c
> index 827c9f2..1d5cc51 100644
> --- a/arch/arm/mach-at91/at91sam9g45_devices.c
> +++ b/arch/arm/mach-at91/at91sam9g45_devices.c
> @@ -981,7 +981,6 @@ static struct resource lcdc_resources[] = {
> };
>
> static struct platform_device at91_lcdc_device = {
> - .name = "atmel_lcdfb",
> .id = 0,
> .dev = {
> .dma_mask = &lcdc_dmamask,
> @@ -997,6 +996,11 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data)
> if (!data)
> return;
>
> + if (cpu_is_at91sam9g45es())
> + at91_lcdc_device.name = "fb-at91sam9g45es";
> + else
> + at91_lcdc_device.name = "fb-at91sam9g45";
> +
> at91_set_A_periph(AT91_PIN_PE0, 0); /* LCDDPWR */
>
> at91_set_A_periph(AT91_PIN_PE2, 0); /* LCDCC */
> diff --git a/arch/arm/mach-at91/at91sam9rl_devices.c b/arch/arm/mach-at91/at91sam9rl_devices.c
> index ddf223f..13cac0a 100644
> --- a/arch/arm/mach-at91/at91sam9rl_devices.c
> +++ b/arch/arm/mach-at91/at91sam9rl_devices.c
> @@ -514,7 +514,7 @@ static struct resource lcdc_resources[] = {
> };
>
> static struct platform_device at91_lcdc_device = {
> - .name = "atmel_lcdfb",
> + .name = "fb-at91sam9rl",
> .id = 0,
> .dev = {
> .dma_mask = &lcdc_dmamask,
> diff --git a/arch/avr32/mach-at32ap/at32ap700x.c b/arch/avr32/mach-at32ap/at32ap700x.c
> index b323d8d..5cdaa07 100644
> --- a/arch/avr32/mach-at32ap/at32ap700x.c
> +++ b/arch/avr32/mach-at32ap/at32ap700x.c
> @@ -1530,6 +1530,8 @@ at32_add_device_lcdc(unsigned int id, struct atmel_lcdfb_info *data,
> memcpy(info, data, sizeof(struct atmel_lcdfb_info));
> info->default_monspecs = monspecs;
>
> + pdev->name = "fb-at32ap";
> +
> platform_device_register(pdev);
> return pdev;
>
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index 347bab2..5ad49ed 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -34,6 +34,81 @@
> #define ATMEL_LCDC_DMA_BURST_LEN 8 /* words */
> #define ATMEL_LCDC_FIFO_SIZE 512 /* words */
>
> +struct atmel_lcdfb_config {
> + bool have_alt_pixclock;
> + bool have_bus_clk;
> + bool have_hozval;
> + bool have_intensity_bit;
> +};
> +
> +static struct atmel_lcdfb_config at91sam9261_config = {
> + .have_bus_clk = true,
> + .have_intensity_bit = true,
> + .have_hozval = true,
> +};
> +
> +static struct atmel_lcdfb_config at91sam9263_config = {
> + .have_intensity_bit = true,
> +};
> +
> +static struct atmel_lcdfb_config at91sam9g10_config = {
> + .have_bus_clk = true,
> + .have_hozval = true,
> +};
> +
> +static struct atmel_lcdfb_config at91sam9g45_config = {
> + .have_alt_pixclock = true,
> +};
> +
> +static struct atmel_lcdfb_config at91sam9g45es_config = {
> +};
> +
> +static struct atmel_lcdfb_config at91sam9rl_config = {
> + .have_intensity_bit = true,
> +};
> +
> +static struct atmel_lcdfb_config at32ap_config = {
> + .have_bus_clk = true,
> + .have_hozval = true,
> +};
> +
> +static const struct platform_device_id atmel_lcdfb_devtypes[] = {
> + {
> + .name = "fb-at91sam9261",
> + .driver_data = (unsigned long)&at91sam9261_config,
> + }, {
> + .name = "fb-at91sam9263",
> + .driver_data = (unsigned long)&at91sam9263_config,
> + }, {
> + .name = "fb-at91sam9g10",
> + .driver_data = (unsigned long)&at91sam9g10_config,
> + }, {
> + .name = "fb-at91sam9g45",
> + .driver_data = (unsigned long)&at91sam9g45_config,
> + }, {
> + .name = "fb-at91sam9g45es",
> + .driver_data = (unsigned long)&at91sam9g45es_config,
> + }, {
> + .name = "fb-at91sam9rl",
> + .driver_data = (unsigned long)&at91sam9rl_config,
> + }, {
> + .name = "fb-at32ap",
> + .driver_data = (unsigned long)&at32ap_config,
> + }, {
> + /* terminator */
> + }
> +};
> +
> +static struct atmel_lcdfb_config *
> +atmel_lcdfb_get_config(struct platform_device *pdev)
> +{
> + unsigned long data;
> +
> + data = platform_get_device_id(pdev)->driver_data;
> +
> + return (struct atmel_lcdfb_config *)data;
> +}
> +
> #if defined(CONFIG_ARCH_AT91)
> #define ATMEL_LCDFB_FBINFO_DEFAULT (FBINFO_DEFAULT \
> | FBINFO_PARTIAL_PAN_OK \
> @@ -199,8 +274,7 @@ static unsigned long compute_hozval(struct atmel_lcdfb_info *sinfo,
> unsigned long lcdcon2;
> unsigned long value;
>
> - if (!(cpu_is_at91sam9261() || cpu_is_at91sam9g10()
> - || cpu_is_at32ap7000()))
> + if (!sinfo->config->have_hozval)
> return xres;
>
> lcdcon2 = lcdc_readl(sinfo, ATMEL_LCDC_LCDCON2);
> @@ -426,7 +500,7 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
> break;
> case 16:
> /* Older SOCs use IBGR:555 rather than BGR:565. */
> - if (sinfo->have_intensity_bit)
> + if (sinfo->config->have_intensity_bit)
> var->green.length = 5;
> else
> var->green.length = 6;
> @@ -534,7 +608,7 @@ static int atmel_lcdfb_set_par(struct fb_info *info)
> /* Now, the LCDC core... */
>
> /* Set pixel clock */
> - if (cpu_is_at91sam9g45() && !cpu_is_at91sam9g45es())
> + if (sinfo->config->have_alt_pixclock)
> pix_factor = 1;
>
> clk_value_khz = clk_get_rate(sinfo->lcdc_clk) / 1000;
> @@ -685,7 +759,7 @@ static int atmel_lcdfb_setcolreg(unsigned int regno, unsigned int red,
>
> case FB_VISUAL_PSEUDOCOLOR:
> if (regno < 256) {
> - if (sinfo->have_intensity_bit) {
> + if (sinfo->config->have_intensity_bit) {
> /* old style I+BGR:555 */
> val = ((red >> 11) & 0x001f);
> val |= ((green >> 6) & 0x03e0);
> @@ -875,10 +949,9 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
> }
> sinfo->info = info;
> sinfo->pdev = pdev;
> - if (cpu_is_at91sam9261() || cpu_is_at91sam9263() ||
> - cpu_is_at91sam9rl()) {
> - sinfo->have_intensity_bit = true;
> - }
> + sinfo->config = atmel_lcdfb_get_config(pdev);
> + if (!sinfo->config)
> + goto free_info;
>
> strcpy(info->fix.id, sinfo->pdev->name);
> info->flags = ATMEL_LCDFB_FBINFO_DEFAULT;
> @@ -889,8 +962,7 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
> info->fix = atmel_lcdfb_fix;
>
> /* Enable LCDC Clocks */
> - if (cpu_is_at91sam9261() || cpu_is_at91sam9g10()
> - || cpu_is_at32ap7000()) {
> + if (sinfo->config->have_bus_clk) {
> sinfo->bus_clk = clk_get(dev, "hck1");
> if (IS_ERR(sinfo->bus_clk)) {
> ret = PTR_ERR(sinfo->bus_clk);
> @@ -1152,7 +1224,7 @@ static struct platform_driver atmel_lcdfb_driver = {
> .remove = __exit_p(atmel_lcdfb_remove),
> .suspend = atmel_lcdfb_suspend,
> .resume = atmel_lcdfb_resume,
> -
> + .id_table = atmel_lcdfb_devtypes,
> .driver = {
> .name = "atmel_lcdfb",
> .owner = THIS_MODULE,
> diff --git a/include/video/atmel_lcdc.h b/include/video/atmel_lcdc.h
> index 8deb226..0f5a2fc 100644
> --- a/include/video/atmel_lcdc.h
> +++ b/include/video/atmel_lcdc.h
> @@ -31,6 +31,7 @@
> #define ATMEL_LCDC_WIRING_BGR 0
> #define ATMEL_LCDC_WIRING_RGB 1
>
> +struct atmel_lcdfb_config;
>
> /* LCD Controller info data structure, stored in device platform_data */
> struct atmel_lcdfb_info {
> @@ -61,7 +62,8 @@ struct atmel_lcdfb_info {
> void (*atmel_lcdfb_power_control)(int on);
> struct fb_monspecs *default_monspecs;
> u32 pseudo_palette[16];
> - bool have_intensity_bit;
> +
> + struct atmel_lcdfb_config *config;
> };
>
> #define ATMEL_LCDC_DMABADDR1 0x00
> --
> 1.8.1.1
>
^ permalink raw reply
* Re: [PATCH v17 4/7] fbmon: add videomode helpers
From: Steffen Trumtrar @ 2013-02-05 18:29 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Mohammed, Afzal', 'Florian Tobias Schandinat',
'Dave Airlie', devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
'Tomi Valkeinen', 'Laurent Pinchart',
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, 'Guennady Liakhovetski',
linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <003401ce005e$af665c50$0e3314f0$%han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Hi!
On Fri, Feb 01, 2013 at 06:29:50PM +0900, Jingoo Han wrote:
> On Friday, January 25, 2013 6:02 PM, Steffen Trumtrar wrote
> >
> > + fbmode->sync = 0;
> > + fbmode->vmode = 0;
> > + if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
> > + fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> > + if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
>
> Um, it seems to be a type. 'H'SYNC -> 'V'SYNC
> Thus, it would be changed as below:
>
> VESA_DMT_HSYNC_HIGH -> VESA_DMT_VSYNC_HIGH
Damn. You are right, that is a typo. But I guess some maintainer (Dave) really,
really wants to take the series now and this can wait for an -rc. No?! ;-)
Thanks,
Steffen
>
> > + fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> > + if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
> > + fbmode->vmode |= FB_VMODE_INTERLACED;
> > + if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
> > + fbmode->vmode |= FB_VMODE_DOUBLE;
> > + fbmode->flag = 0;
> > +
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Alexander Holler @ 2013-02-05 17:36 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, linux-kernel, linux-fbdev,
Florian Tobias Schandinat, Bernie Thompson, Steve Glendinning,
stable
In-Reply-To: <20130205172245.GA1426@kroah.com>
Am 05.02.2013 18:22, schrieb Greg KH:
> On Tue, Feb 05, 2013 at 08:08:28AM +0100, Alexander Holler wrote:
>> Am 04.02.2013 20:25, schrieb Greg KH:
>>> Where was that urb when the disconnect happened? The USB core should
>>> call your urb callback for any outstanding urbs at that point in time,
>>> with the proper error flag being set, are you handling that properly?
>>
>> I don't know where that urb is as I don't handle it.
>
> What do you mean by that? The urb is being sent back to your driver,
> right? If not, that's a bug, but please be sure that your urb callback
> isn't really being called.
I meant it isn't _my_ driver. ;)
I'm just trying to add some würgarounds without having the need to
rewrite the whole driver.
In regard to that "urb missing problem", I think I've just named it
wrong and the actual problem is a race-condition between the semaphore
handling (which is used to keep track of the urbs) and the urb handling
inside the driver.
But I've just switched to udl (instead of udlfb) and will see if I can
fix the bugs there to make it usable as a console. udl is a rewrite of
udlfb with some additional features (e.g. drm), so hopefully fixing the
remaining problems there will require less work.
Regards,
Alexander
^ permalink raw reply
* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Greg KH @ 2013-02-05 17:22 UTC (permalink / raw)
To: Alexander Holler
Cc: Andrew Morton, linux-kernel, linux-fbdev,
Florian Tobias Schandinat, Bernie Thompson, Steve Glendinning,
stable
In-Reply-To: <5110AFEC.8020406@ahsoftware.de>
On Tue, Feb 05, 2013 at 08:08:28AM +0100, Alexander Holler wrote:
> Am 04.02.2013 20:25, schrieb Greg KH:
> > On Mon, Feb 04, 2013 at 08:17:04PM +0100, Alexander Holler wrote:
> >> Am 04.02.2013 13:05, schrieb Alexander Holler:
> >>> Am 04.02.2013 02:14, schrieb Greg KH:
> >>>
> >>>> So you are right in that your driver will wait for forever for a
> >>>> disconnect() to happen, as it will never be called. I don't understand
> >>>> the problem that this is causing when it happens. What's wrong with
> >>>> udlfb that having the cpu suddently reset as the powerdown happened
> >>>> without it knowing about it?
> >>>
> >>> There is nothing wrong with that. I've just explained why a problem
> >>> doesn't occur on shutdown but on disconnect (of the device).
> >>
> >> Maybe my explanation before was just to long and I try to explain it
> >> a bit shorter:
> >>
> >> If a device gets disconnected, the disconnect in udlfb might wait
> >> forever in down_interruptible() (because it waits for an urb it
> >> never receives). This even prevents a shutdown afterwards, because
> >> that down_interruptible() never receives a signal (at shutdown,
> >> because kernel threads don't get such).
> >
> > Where was that urb when the disconnect happened? The USB core should
> > call your urb callback for any outstanding urbs at that point in time,
> > with the proper error flag being set, are you handling that properly?
>
> I don't know where that urb is as I don't handle it.
What do you mean by that? The urb is being sent back to your driver,
right? If not, that's a bug, but please be sure that your urb callback
isn't really being called.
thanks,
greg k-h
^ permalink raw reply
* [PATCH 5/5] ARM: at91/avr32/atmel_lcdfb: replace cpu_is macros with device-id table
From: Johan Hovold @ 2013-02-05 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1360071315-4032-1-git-send-email-jhovold@gmail.com>
Remove cpu_is macros from atmel lcdfb driver and use platform-device-id
table to determine platform configuration parameters.
The currently used configuration parameters are:
have_alt_pixclock
- SOC uses an alternate pixel-clock calculation formula (at91sam9g45
non-ES)
have_bus_clk
- SOC has bus clock hck1 (at91sam9261, at921sam9g10 and at32ap)
have_hozval
- SOC has a HOZVAL field in LCDFRMCFG which is used to determine the
linesize for STN displays (at91sam9261, at921sam9g10 and at32ap)
have_intensity_bit
- SOC uses IBGR:555 rather than BGR:565 16-bit pixel layout
(at91sam9261, at91sam9263 and at91sam9rl)
Tested on at91sam9g45, compile-tested for other AT91 SOCs, and untested
for AVR32.
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
arch/arm/mach-at91/at91sam9261_devices.c | 6 +-
arch/arm/mach-at91/at91sam9263_devices.c | 2 +-
arch/arm/mach-at91/at91sam9g45_devices.c | 6 +-
arch/arm/mach-at91/at91sam9rl_devices.c | 2 +-
arch/avr32/mach-at32ap/at32ap700x.c | 2 +
drivers/video/atmel_lcdfb.c | 96 ++++++++++++++++++++++++++++----
include/video/atmel_lcdc.h | 4 +-
7 files changed, 101 insertions(+), 17 deletions(-)
diff --git a/arch/arm/mach-at91/at91sam9261_devices.c b/arch/arm/mach-at91/at91sam9261_devices.c
index 92e0f86..01647cb 100644
--- a/arch/arm/mach-at91/at91sam9261_devices.c
+++ b/arch/arm/mach-at91/at91sam9261_devices.c
@@ -488,7 +488,6 @@ static struct resource lcdc_resources[] = {
};
static struct platform_device at91_lcdc_device = {
- .name = "atmel_lcdfb",
.id = 0,
.dev = {
.dma_mask = &lcdc_dmamask,
@@ -505,6 +504,11 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data)
return;
}
+ if (cpu_is_at91sam9g10())
+ at91_lcdc_device.name = "fb-at91sam9g10";
+ else
+ at91_lcdc_device.name = "fb-at91sam9261";
+
#if defined(CONFIG_FB_ATMEL_STN)
at91_set_A_periph(AT91_PIN_PB0, 0); /* LCDVSYNC */
at91_set_A_periph(AT91_PIN_PB1, 0); /* LCDHSYNC */
diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
index ed666f5..a34f39a 100644
--- a/arch/arm/mach-at91/at91sam9263_devices.c
+++ b/arch/arm/mach-at91/at91sam9263_devices.c
@@ -848,7 +848,7 @@ static struct resource lcdc_resources[] = {
};
static struct platform_device at91_lcdc_device = {
- .name = "atmel_lcdfb",
+ .name = "fb-at91sam9263",
.id = 0,
.dev = {
.dma_mask = &lcdc_dmamask,
diff --git a/arch/arm/mach-at91/at91sam9g45_devices.c b/arch/arm/mach-at91/at91sam9g45_devices.c
index 827c9f2..1d5cc51 100644
--- a/arch/arm/mach-at91/at91sam9g45_devices.c
+++ b/arch/arm/mach-at91/at91sam9g45_devices.c
@@ -981,7 +981,6 @@ static struct resource lcdc_resources[] = {
};
static struct platform_device at91_lcdc_device = {
- .name = "atmel_lcdfb",
.id = 0,
.dev = {
.dma_mask = &lcdc_dmamask,
@@ -997,6 +996,11 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data)
if (!data)
return;
+ if (cpu_is_at91sam9g45es())
+ at91_lcdc_device.name = "fb-at91sam9g45es";
+ else
+ at91_lcdc_device.name = "fb-at91sam9g45";
+
at91_set_A_periph(AT91_PIN_PE0, 0); /* LCDDPWR */
at91_set_A_periph(AT91_PIN_PE2, 0); /* LCDCC */
diff --git a/arch/arm/mach-at91/at91sam9rl_devices.c b/arch/arm/mach-at91/at91sam9rl_devices.c
index ddf223f..13cac0a 100644
--- a/arch/arm/mach-at91/at91sam9rl_devices.c
+++ b/arch/arm/mach-at91/at91sam9rl_devices.c
@@ -514,7 +514,7 @@ static struct resource lcdc_resources[] = {
};
static struct platform_device at91_lcdc_device = {
- .name = "atmel_lcdfb",
+ .name = "fb-at91sam9rl",
.id = 0,
.dev = {
.dma_mask = &lcdc_dmamask,
diff --git a/arch/avr32/mach-at32ap/at32ap700x.c b/arch/avr32/mach-at32ap/at32ap700x.c
index b323d8d..5cdaa07 100644
--- a/arch/avr32/mach-at32ap/at32ap700x.c
+++ b/arch/avr32/mach-at32ap/at32ap700x.c
@@ -1530,6 +1530,8 @@ at32_add_device_lcdc(unsigned int id, struct atmel_lcdfb_info *data,
memcpy(info, data, sizeof(struct atmel_lcdfb_info));
info->default_monspecs = monspecs;
+ pdev->name = "fb-at32ap";
+
platform_device_register(pdev);
return pdev;
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 347bab2..5ad49ed 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -34,6 +34,81 @@
#define ATMEL_LCDC_DMA_BURST_LEN 8 /* words */
#define ATMEL_LCDC_FIFO_SIZE 512 /* words */
+struct atmel_lcdfb_config {
+ bool have_alt_pixclock;
+ bool have_bus_clk;
+ bool have_hozval;
+ bool have_intensity_bit;
+};
+
+static struct atmel_lcdfb_config at91sam9261_config = {
+ .have_bus_clk = true,
+ .have_intensity_bit = true,
+ .have_hozval = true,
+};
+
+static struct atmel_lcdfb_config at91sam9263_config = {
+ .have_intensity_bit = true,
+};
+
+static struct atmel_lcdfb_config at91sam9g10_config = {
+ .have_bus_clk = true,
+ .have_hozval = true,
+};
+
+static struct atmel_lcdfb_config at91sam9g45_config = {
+ .have_alt_pixclock = true,
+};
+
+static struct atmel_lcdfb_config at91sam9g45es_config = {
+};
+
+static struct atmel_lcdfb_config at91sam9rl_config = {
+ .have_intensity_bit = true,
+};
+
+static struct atmel_lcdfb_config at32ap_config = {
+ .have_bus_clk = true,
+ .have_hozval = true,
+};
+
+static const struct platform_device_id atmel_lcdfb_devtypes[] = {
+ {
+ .name = "fb-at91sam9261",
+ .driver_data = (unsigned long)&at91sam9261_config,
+ }, {
+ .name = "fb-at91sam9263",
+ .driver_data = (unsigned long)&at91sam9263_config,
+ }, {
+ .name = "fb-at91sam9g10",
+ .driver_data = (unsigned long)&at91sam9g10_config,
+ }, {
+ .name = "fb-at91sam9g45",
+ .driver_data = (unsigned long)&at91sam9g45_config,
+ }, {
+ .name = "fb-at91sam9g45es",
+ .driver_data = (unsigned long)&at91sam9g45es_config,
+ }, {
+ .name = "fb-at91sam9rl",
+ .driver_data = (unsigned long)&at91sam9rl_config,
+ }, {
+ .name = "fb-at32ap",
+ .driver_data = (unsigned long)&at32ap_config,
+ }, {
+ /* terminator */
+ }
+};
+
+static struct atmel_lcdfb_config *
+atmel_lcdfb_get_config(struct platform_device *pdev)
+{
+ unsigned long data;
+
+ data = platform_get_device_id(pdev)->driver_data;
+
+ return (struct atmel_lcdfb_config *)data;
+}
+
#if defined(CONFIG_ARCH_AT91)
#define ATMEL_LCDFB_FBINFO_DEFAULT (FBINFO_DEFAULT \
| FBINFO_PARTIAL_PAN_OK \
@@ -199,8 +274,7 @@ static unsigned long compute_hozval(struct atmel_lcdfb_info *sinfo,
unsigned long lcdcon2;
unsigned long value;
- if (!(cpu_is_at91sam9261() || cpu_is_at91sam9g10()
- || cpu_is_at32ap7000()))
+ if (!sinfo->config->have_hozval)
return xres;
lcdcon2 = lcdc_readl(sinfo, ATMEL_LCDC_LCDCON2);
@@ -426,7 +500,7 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
break;
case 16:
/* Older SOCs use IBGR:555 rather than BGR:565. */
- if (sinfo->have_intensity_bit)
+ if (sinfo->config->have_intensity_bit)
var->green.length = 5;
else
var->green.length = 6;
@@ -534,7 +608,7 @@ static int atmel_lcdfb_set_par(struct fb_info *info)
/* Now, the LCDC core... */
/* Set pixel clock */
- if (cpu_is_at91sam9g45() && !cpu_is_at91sam9g45es())
+ if (sinfo->config->have_alt_pixclock)
pix_factor = 1;
clk_value_khz = clk_get_rate(sinfo->lcdc_clk) / 1000;
@@ -685,7 +759,7 @@ static int atmel_lcdfb_setcolreg(unsigned int regno, unsigned int red,
case FB_VISUAL_PSEUDOCOLOR:
if (regno < 256) {
- if (sinfo->have_intensity_bit) {
+ if (sinfo->config->have_intensity_bit) {
/* old style I+BGR:555 */
val = ((red >> 11) & 0x001f);
val |= ((green >> 6) & 0x03e0);
@@ -875,10 +949,9 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
}
sinfo->info = info;
sinfo->pdev = pdev;
- if (cpu_is_at91sam9261() || cpu_is_at91sam9263() ||
- cpu_is_at91sam9rl()) {
- sinfo->have_intensity_bit = true;
- }
+ sinfo->config = atmel_lcdfb_get_config(pdev);
+ if (!sinfo->config)
+ goto free_info;
strcpy(info->fix.id, sinfo->pdev->name);
info->flags = ATMEL_LCDFB_FBINFO_DEFAULT;
@@ -889,8 +962,7 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
info->fix = atmel_lcdfb_fix;
/* Enable LCDC Clocks */
- if (cpu_is_at91sam9261() || cpu_is_at91sam9g10()
- || cpu_is_at32ap7000()) {
+ if (sinfo->config->have_bus_clk) {
sinfo->bus_clk = clk_get(dev, "hck1");
if (IS_ERR(sinfo->bus_clk)) {
ret = PTR_ERR(sinfo->bus_clk);
@@ -1152,7 +1224,7 @@ static struct platform_driver atmel_lcdfb_driver = {
.remove = __exit_p(atmel_lcdfb_remove),
.suspend = atmel_lcdfb_suspend,
.resume = atmel_lcdfb_resume,
-
+ .id_table = atmel_lcdfb_devtypes,
.driver = {
.name = "atmel_lcdfb",
.owner = THIS_MODULE,
diff --git a/include/video/atmel_lcdc.h b/include/video/atmel_lcdc.h
index 8deb226..0f5a2fc 100644
--- a/include/video/atmel_lcdc.h
+++ b/include/video/atmel_lcdc.h
@@ -31,6 +31,7 @@
#define ATMEL_LCDC_WIRING_BGR 0
#define ATMEL_LCDC_WIRING_RGB 1
+struct atmel_lcdfb_config;
/* LCD Controller info data structure, stored in device platform_data */
struct atmel_lcdfb_info {
@@ -61,7 +62,8 @@ struct atmel_lcdfb_info {
void (*atmel_lcdfb_power_control)(int on);
struct fb_monspecs *default_monspecs;
u32 pseudo_palette[16];
- bool have_intensity_bit;
+
+ struct atmel_lcdfb_config *config;
};
#define ATMEL_LCDC_DMABADDR1 0x00
--
1.8.1.1
^ permalink raw reply related
* [PATCH 4/5] atmel_lcdfb: move lcdcon2 register access to compute_hozval
From: Johan Hovold @ 2013-02-05 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1360071315-4032-1-git-send-email-jhovold@gmail.com>
Pass atmel_lcd_info structure to compute_hozval and only do the register
access on SOCs that actually use it.
This will also simplify the removal of the cpu_is macros.
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/video/atmel_lcdfb.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 93910e3..347bab2 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -193,14 +193,17 @@ static struct fb_fix_screeninfo atmel_lcdfb_fix __initdata = {
.accel = FB_ACCEL_NONE,
};
-static unsigned long compute_hozval(unsigned long xres, unsigned long lcdcon2)
+static unsigned long compute_hozval(struct atmel_lcdfb_info *sinfo,
+ unsigned long xres)
{
+ unsigned long lcdcon2;
unsigned long value;
if (!(cpu_is_at91sam9261() || cpu_is_at91sam9g10()
|| cpu_is_at32ap7000()))
return xres;
+ lcdcon2 = lcdc_readl(sinfo, ATMEL_LCDC_LCDCON2);
value = xres;
if ((lcdcon2 & ATMEL_LCDC_DISTYPE) != ATMEL_LCDC_DISTYPE_TFT) {
/* STN display */
@@ -590,8 +593,7 @@ static int atmel_lcdfb_set_par(struct fb_info *info)
lcdc_writel(sinfo, ATMEL_LCDC_TIM2, value);
/* Horizontal value (aka line size) */
- hozval_linesz = compute_hozval(info->var.xres,
- lcdc_readl(sinfo, ATMEL_LCDC_LCDCON2));
+ hozval_linesz = compute_hozval(sinfo, info->var.xres);
/* Display size */
value = (hozval_linesz - 1) << ATMEL_LCDC_HOZVAL_OFFSET;
--
1.8.1.1
^ permalink raw reply related
* [PATCH 3/5] atmel_lcdfb: remove unsupported 15-bpp mode
From: Johan Hovold @ 2013-02-05 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1360071315-4032-1-git-send-email-jhovold@gmail.com>
Since commit 787f9fd23283 ("atmel_lcdfb: support 16bit BGR:565 mode,
remove unsupported 15bit modes") atmel_lcdfb_check_var does not accept
15-bpp mode so remove it from atmel_lcdfb_set_par as well.
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/video/atmel_lcdfb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 025428e..93910e3 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -567,7 +567,6 @@ static int atmel_lcdfb_set_par(struct fb_info *info)
case 2: value |= ATMEL_LCDC_PIXELSIZE_2; break;
case 4: value |= ATMEL_LCDC_PIXELSIZE_4; break;
case 8: value |= ATMEL_LCDC_PIXELSIZE_8; break;
- case 15: /* fall through */
case 16: value |= ATMEL_LCDC_PIXELSIZE_16; break;
case 24: value |= ATMEL_LCDC_PIXELSIZE_24; break;
case 32: value |= ATMEL_LCDC_PIXELSIZE_32; break;
--
1.8.1.1
^ permalink raw reply related
* [PATCH 2/5] ARM: at91/neocore926: fix LCD-wiring mode
From: Johan Hovold @ 2013-02-05 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1360071315-4032-1-git-send-email-jhovold@gmail.com>
Fix regression introduced by commit 787f9fd23283 ("atmel_lcdfb: support
16bit BGR:565 mode, remove unsupported 15bit modes") which broke 16-bpp
modes for older SOCs which use IBGR:555 (msb is intensity) rather than
BGR:565.
The above commit also removed the RGB:555-wiring hack without fixing the
neocore926 board which used it. Fix by specifying RGB-wiring and let the
driver handle the final SOC-dependant layout.
Remove the no longer used ATMEL_LCDC_WIRING_RGB555 define.
Compile-only tested.
Cc: <stable@vger.kernel.org>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
arch/arm/mach-at91/board-neocore926.c | 2 +-
include/video/atmel_lcdc.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/mach-at91/board-neocore926.c b/arch/arm/mach-at91/board-neocore926.c
index bc7a1c4..4726297 100644
--- a/arch/arm/mach-at91/board-neocore926.c
+++ b/arch/arm/mach-at91/board-neocore926.c
@@ -266,7 +266,7 @@ static struct atmel_lcdfb_info __initdata neocore926_lcdc_data = {
.default_monspecs = &at91fb_default_monspecs,
.atmel_lcdfb_power_control = at91_lcdc_power_control,
.guard_time = 1,
- .lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB555,
+ .lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB,
};
#else
diff --git a/include/video/atmel_lcdc.h b/include/video/atmel_lcdc.h
index 5f0e234..8deb226 100644
--- a/include/video/atmel_lcdc.h
+++ b/include/video/atmel_lcdc.h
@@ -30,7 +30,6 @@
*/
#define ATMEL_LCDC_WIRING_BGR 0
#define ATMEL_LCDC_WIRING_RGB 1
-#define ATMEL_LCDC_WIRING_RGB555 2
/* LCD Controller info data structure, stored in device platform_data */
--
1.8.1.1
^ permalink raw reply related
* [PATCH 1/5] atmel_lcdfb: fix 16-bpp modes on older SOCs
From: Johan Hovold @ 2013-02-05 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1360071315-4032-1-git-send-email-jhovold@gmail.com>
Fix regression introduced by commit 787f9fd23283 ("atmel_lcdfb: support
16bit BGR:565 mode, remove unsupported 15bit modes") which broke 16-bpp
modes for older SOCs which use IBGR:555 (msb is intensity) rather
than BGR:565.
Use SOC-type to determine the pixel layout.
Tested on at91sam9263 and at91sam9g45.
Cc: <stable@vger.kernel.org>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/video/atmel_lcdfb.c | 22 +++++++++++++++-------
include/video/atmel_lcdc.h | 1 +
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 12cf5f3..025428e 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -422,17 +422,22 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
= var->bits_per_pixel;
break;
case 16:
+ /* Older SOCs use IBGR:555 rather than BGR:565. */
+ if (sinfo->have_intensity_bit)
+ var->green.length = 5;
+ else
+ var->green.length = 6;
+
if (sinfo->lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB) {
- /* RGB:565 mode */
- var->red.offset = 11;
+ /* RGB:5X5 mode */
+ var->red.offset = var->green.length + 5;
var->blue.offset = 0;
} else {
- /* BGR:565 mode */
+ /* BGR:5X5 mode */
var->red.offset = 0;
- var->blue.offset = 11;
+ var->blue.offset = var->green.length + 5;
}
var->green.offset = 5;
- var->green.length = 6;
var->red.length = var->blue.length = 5;
break;
case 32:
@@ -679,8 +684,7 @@ static int atmel_lcdfb_setcolreg(unsigned int regno, unsigned int red,
case FB_VISUAL_PSEUDOCOLOR:
if (regno < 256) {
- if (cpu_is_at91sam9261() || cpu_is_at91sam9263()
- || cpu_is_at91sam9rl()) {
+ if (sinfo->have_intensity_bit) {
/* old style I+BGR:555 */
val = ((red >> 11) & 0x001f);
val |= ((green >> 6) & 0x03e0);
@@ -870,6 +874,10 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
}
sinfo->info = info;
sinfo->pdev = pdev;
+ if (cpu_is_at91sam9261() || cpu_is_at91sam9263() ||
+ cpu_is_at91sam9rl()) {
+ sinfo->have_intensity_bit = true;
+ }
strcpy(info->fix.id, sinfo->pdev->name);
info->flags = ATMEL_LCDFB_FBINFO_DEFAULT;
diff --git a/include/video/atmel_lcdc.h b/include/video/atmel_lcdc.h
index 28447f1..5f0e234 100644
--- a/include/video/atmel_lcdc.h
+++ b/include/video/atmel_lcdc.h
@@ -62,6 +62,7 @@ struct atmel_lcdfb_info {
void (*atmel_lcdfb_power_control)(int on);
struct fb_monspecs *default_monspecs;
u32 pseudo_palette[16];
+ bool have_intensity_bit;
};
#define ATMEL_LCDC_DMABADDR1 0x00
--
1.8.1.1
^ permalink raw reply related
* [PATCH 0/5] atmel_lcdfb: regression fixes and cpu_is removal
From: Johan Hovold @ 2013-02-05 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130129135435.GN7360@game.jcrosoft.org>
The first three patches are resends of two regression fixes and one clean up
posted in December (with previous acked-bys included). The regression fixes are
kept minimal and are tagged for stable.
The last two patches replace all uses of cpu_is macros in atmel_lcdfb with a
platform-device-id table and static configurations.
Thanks,
Johan
Johan Hovold (5):
atmel_lcdfb: fix 16-bpp modes on older SOCs
ARM: at91/neocore926: fix LCD-wiring mode
atmel_lcdfb: remove unsupported 15-bpp mode
atmel_lcdfb: move lcdcon2 register access to compute_hozval
ARM: at91/avr32/atmel_lcdfb: replace cpu_is macros with device-id
table
arch/arm/mach-at91/at91sam9261_devices.c | 6 +-
arch/arm/mach-at91/at91sam9263_devices.c | 2 +-
arch/arm/mach-at91/at91sam9g45_devices.c | 6 +-
arch/arm/mach-at91/at91sam9rl_devices.c | 2 +-
arch/arm/mach-at91/board-neocore926.c | 2 +-
arch/avr32/mach-at32ap/at32ap700x.c | 2 +
drivers/video/atmel_lcdfb.c | 115 ++++++++++++++++++++++++++-----
include/video/atmel_lcdc.h | 4 +-
8 files changed, 116 insertions(+), 23 deletions(-)
--
1.8.1.1
^ permalink raw reply
* Re: Revert "console: implement lockdep support for console_lock"
From: Sedat Dilek @ 2013-02-05 10:50 UTC (permalink / raw)
To: Daniel Vetter
Cc: Dave Airlie, Linus Torvalds, linux-fbdev, LKML, linux-next,
Andrew Morton, Stephen Rothwell
In-Reply-To: <CA+icZUU4JMfws_jFpoCaXnSNJeigONSEik_oGSa_ZQ=uJ_GJOA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1863 bytes --]
On Tue, Feb 5, 2013 at 11:47 AM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> On Fri, Feb 1, 2013 at 9:42 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> On Fri, Feb 1, 2013 at 9:21 AM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>>> people having the fbcon-locking-fixes [1] in their local GIT tree can
>>> revert this change?
>>
>> Yeah, if you have all the fixes reverting this is fine and appreciated
>> to increase testing. Dave will probably push the revert himself to
>> drm-next soon.
>
> When will that happen: drm-next inclusion?
>
> Just FYI:
> Pulling fbcon-locking-fixes into Linux-Next (next-20130205) shows some
> ugly UTF-8 mismatch, means "fb: rework locking to fix lock ordering on
> takeover" is not the same patch as in -next.
> Just wanna let you know.
>
> It would be good to inform Andrew and Stephen when this happened, so
> Andrew can drop the two fb-fixes from his mmotm-tree.
>
Attached the UTF-8 "mismatch" in vt.c.
- Sedat -
> Thanks!
>
> - Sedat -
>
>> -Daniel
>>
>>>
>>> commit ff0d05bf73620eb7dc8aee7423e992ef87870bdf
>>> Refs: v3.8-rc5-194-gff0d05b
>>> Author: Dave Airlie <airlied@gmail.com>
>>> AuthorDate: Thu Jan 31 14:27:03 2013 +1100
>>> Commit: Dave Airlie <airlied@gmail.com>
>>> CommitDate: Thu Jan 31 15:46:56 2013 +1100
>>>
>>> Revert "console: implement lockdep support for console_lock"
>>>
>>> This reverts commit daee779718a319ff9f83e1ba3339334ac650bb22.
>>>
>>> I'll requeue this after the console locking fixes, so lockdep
>>> is useful again for people until fbcon is fixed.
>>>
>>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>>>
>>> Thanks!
>>>
>>> Regards,
>>> - Sedat
>>>
>>> [1] http://cgit.freedesktop.org/~airlied/linux/log/?h=fbcon-locking-fixes
>>
>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[-- Attachment #2: VT_C_UTF-8_CRAP.diff --]
[-- Type: application/octet-stream, Size: 1773 bytes --]
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 56dd69c..86b76f4 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3656,7 +3656,7 @@ EXPORT_SYMBOL_GPL(do_unregister_con_driver);
* when a driver wants to take over some existing consoles
* and become default driver for newly opened ones.
*
- * take_over_console is basically a register followed by unbind
+ * take_over_console is basically a register followed by unbind
*/
int do_take_over_console(const struct consw *csw, int first, int last, int deflt)
{
@@ -3666,7 +3666,7 @@ int do_take_over_console(const struct consw *csw, int first, int last, int deflt
/*
* If we get an busy error we still want to bind the console driver
* and return success, as we may have unbound the console driver
- * but not unregistered it.
+ * but not unregistered it.
*/
if (err == -EBUSY)
err = 0;
@@ -3682,7 +3682,7 @@ EXPORT_SYMBOL_GPL(do_take_over_console);
* when a driver wants to take over some existing consoles
* and become default driver for newly opened ones.
*
- * take_over_console is basically a register followed by unbind
+ * take_over_console is basically a register followed by unbind
*/
int take_over_console(const struct consw *csw, int first, int last, int deflt)
{
@@ -3692,7 +3692,7 @@ int take_over_console(const struct consw *csw, int first, int last, int deflt)
/*
* If we get an busy error we still want to bind the console driver
* and return success, as we may have unbound the console driver
- * but not unregistered it.
+ * but not unregistered it.
*/
if (err == -EBUSY)
err = 0;
^ permalink raw reply related
* Re: Revert "console: implement lockdep support for console_lock"
From: Sedat Dilek @ 2013-02-05 10:47 UTC (permalink / raw)
To: Daniel Vetter
Cc: Dave Airlie, Linus Torvalds, linux-fbdev, LKML, linux-next,
Andrew Morton, Stephen Rothwell
In-Reply-To: <CAKMK7uEFCm5JPD9bg4uO9K_q-w0=9GWzRLoB9H-_=ZR7NxA9=A@mail.gmail.com>
On Fri, Feb 1, 2013 at 9:42 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Fri, Feb 1, 2013 at 9:21 AM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>> people having the fbcon-locking-fixes [1] in their local GIT tree can
>> revert this change?
>
> Yeah, if you have all the fixes reverting this is fine and appreciated
> to increase testing. Dave will probably push the revert himself to
> drm-next soon.
When will that happen: drm-next inclusion?
Just FYI:
Pulling fbcon-locking-fixes into Linux-Next (next-20130205) shows some
ugly UTF-8 mismatch, means "fb: rework locking to fix lock ordering on
takeover" is not the same patch as in -next.
Just wanna let you know.
It would be good to inform Andrew and Stephen when this happened, so
Andrew can drop the two fb-fixes from his mmotm-tree.
Thanks!
- Sedat -
> -Daniel
>
>>
>> commit ff0d05bf73620eb7dc8aee7423e992ef87870bdf
>> Refs: v3.8-rc5-194-gff0d05b
>> Author: Dave Airlie <airlied@gmail.com>
>> AuthorDate: Thu Jan 31 14:27:03 2013 +1100
>> Commit: Dave Airlie <airlied@gmail.com>
>> CommitDate: Thu Jan 31 15:46:56 2013 +1100
>>
>> Revert "console: implement lockdep support for console_lock"
>>
>> This reverts commit daee779718a319ff9f83e1ba3339334ac650bb22.
>>
>> I'll requeue this after the console locking fixes, so lockdep
>> is useful again for people until fbcon is fixed.
>>
>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>>
>> Thanks!
>>
>> Regards,
>> - Sedat
>>
>> [1] http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH] backlight: add an AS3711 PMIC backlight driver
From: Guennadi Liakhovetski @ 2013-02-05 9:08 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Andrew Morton', linux-kernel, linux-fbdev, linux-sh,
'Magnus Damm', 'Richard Purdie'
In-Reply-To: <00fe01cdfcf1$dcfdc970$96f95c50$%han@samsung.com>
Hi Richard
Could you tell us your opinion on this:
On Mon, 28 Jan 2013, Jingoo Han wrote:
> On Friday, January 25, 2013 4:49 PM, Guennadi Liakhovetski wrote
> >
> > Hi Jingoo Han
> >
> > On Fri, 25 Jan 2013, Jingoo Han wrote:
> >
> > > On Wednesday, January 09, 2013 2:55 AM, Guennadi Liakhovetski wrote
> > > >
> > > > This is an initial commit of a backlight driver, using step-up DCDC power
> > > > supplies on AS3711 PMIC. Only one mode has actually been tested, several
> > > > further modes have been implemented "dry," but disabled to avoid accidental
> > > > hardware damage. Anyone wishing to use any of those modes will have to
> > > > modify the driver.
> > > >
> > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > >
> > > CC'ed Andrew Morton.
> > >
> > > Hi Guennadi Liakhovetski,
> > >
> > > I have reviewed this patch with AS3711 datasheet.
> > > I cannot find any problems. It looks good.
> >
> > Thanks for the review.
> >
> > > However, some hardcoded numbers need to be changed
> > > to the bit definitions.
> >
> > Which specific hardcoded values do you mean? A while ago in a discussion
> > on one of kernel mailing lists a conclusion has been made, that macros do
> > not always improve code readability. E.g. in a statement like this
> >
> > + case AS3711_SU2_CURR_AUTO:
> > + if (pdata->su2_auto_curr1)
> > + ctl = 2;
> > + if (pdata->su2_auto_curr2)
> > + ctl |= 8;
> > + if (pdata->su2_auto_curr3)
> > + ctl |= 0x20;
> >
> > making it
> >
> > + case AS3711_SU2_CURR_AUTO:
> > + if (pdata->su2_auto_curr1)
> > + ctl = SU2_AUTO_CURR1;
> >
> > would hardly make it more readable. IMHO it is already pretty clear, that
> > bit 1 enables the current-1 sink. As long as these fields are only used at
> > one location in the driver (and they are), using a macro and defining it
> > elsewhere only makes it harder to see actual values. Speaking of this, a
> > comment like
>
> I don't think so. Some people feel that it is not clear a bit.
> Of course, I know what you mean.
> Also, your comment is reasonable.
> However, personally, I prefer the latter.
> Because, I think that the meaning of bits is more important than
> actual bits. In the latter case, we can notice the meaning of bits
> more easily.
Do you also find preferable to use symbolic names for every single bit,
occurring in a driver, or you agree, that excessive use of such macros can
only needlessly clutter the code?
Thanks
Guennadi
> Best regards,
> Jingoo Han
>
> >
> > /*
> > * Select, which current sinks shall be used for automatic
> > * feedback selection
> > */
> >
> > would help much more than any macro names. But as it stands, I think the
> > current version is also possible to understand :-) If desired, however,
> > comments can be added in an incremental patch
>
> >
> > Thanks
> > Guennadi
> >
> > > Acked-by: Jingoo Han <jg1.han@samsung.com>
> > >
> > >
> > > Best regards,
> > > Jingoo Han
> > >
> > > > ---
> > > >
> > > > Tested on sh73a0-based kzm9g board. As the commit message says, only one
> > > > mode has been tested and is enabled. That mode copies the sample code from
> > > > the manufacturer. Deviations from that code proved to be fatal for the
> > > > hardware...
> > > >
> > > > drivers/video/backlight/Kconfig | 7 +
> > > > drivers/video/backlight/Makefile | 1 +
> > > > drivers/video/backlight/as3711_bl.c | 379 +++++++++++++++++++++++++++++++++++
> > > > 3 files changed, 387 insertions(+), 0 deletions(-)
> > > > create mode 100644 drivers/video/backlight/as3711_bl.c
> > > >
> > > > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> > > > index 765a945..6ef0ede 100644
> > > > --- a/drivers/video/backlight/Kconfig
> > > > +++ b/drivers/video/backlight/Kconfig
> > > > @@ -390,6 +390,13 @@ config BACKLIGHT_TPS65217
> > > > If you have a Texas Instruments TPS65217 say Y to enable the
> > > > backlight driver.
> > > >
> > > > +config BACKLIGHT_AS3711
> > > > + tristate "AS3711 Backlight"
> > > > + depends on BACKLIGHT_CLASS_DEVICE && MFD_AS3711
> > > > + help
> > > > + If you have an Austrian Microsystems AS3711 say Y to enable the
> > > > + backlight driver.
> > > > +
> > > > endif # BACKLIGHT_CLASS_DEVICE
> > > >
> > > > endif # BACKLIGHT_LCD_SUPPORT
> > > > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> > > > index e7ce729..d3e188f 100644
> > > > --- a/drivers/video/backlight/Makefile
> > > > +++ b/drivers/video/backlight/Makefile
> > > > @@ -45,3 +45,4 @@ obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o
> > > > obj-$(CONFIG_BACKLIGHT_AAT2870) += aat2870_bl.o
> > > > obj-$(CONFIG_BACKLIGHT_OT200) += ot200_bl.o
> > > > obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
> > > > +obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o
> > > > diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c
> > > > new file mode 100644
> > > > index 0000000..c6bc65d
> > > > --- /dev/null
> > > > +++ b/drivers/video/backlight/as3711_bl.c
> > > > @@ -0,0 +1,379 @@
> > > > +/*
> > > > + * AS3711 PMIC backlight driver, using DCDC Step Up Converters
> > > > + *
> > > > + * Copyright (C) 2012 Renesas Electronics Corporation
> > > > + * Author: Guennadi Liakhovetski, <g.liakhovetski@gmx.de>
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the version 2 of the GNU General Public License as
> > > > + * published by the Free Software Foundation
> > > > + */
> > > > +
> > > > +#include <linux/backlight.h>
> > > > +#include <linux/delay.h>
> > > > +#include <linux/device.h>
> > > > +#include <linux/err.h>
> > > > +#include <linux/fb.h>
> > > > +#include <linux/kernel.h>
> > > > +#include <linux/mfd/as3711.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/regmap.h>
> > > > +#include <linux/slab.h>
> > > > +
> > > > +enum as3711_bl_type {
> > > > + AS3711_BL_SU1,
> > > > + AS3711_BL_SU2,
> > > > +};
> > > > +
> > > > +struct as3711_bl_data {
> > > > + bool powered;
> > > > + const char *fb_name;
> > > > + struct device *fb_dev;
> > > > + enum as3711_bl_type type;
> > > > + int brightness;
> > > > + struct backlight_device *bl;
> > > > +};
> > > > +
> > > > +struct as3711_bl_supply {
> > > > + struct as3711_bl_data su1;
> > > > + struct as3711_bl_data su2;
> > > > + const struct as3711_bl_pdata *pdata;
> > > > + struct as3711 *as3711;
> > > > +};
> > > > +
> > > > +static struct as3711_bl_supply *to_supply(struct as3711_bl_data *su)
> > > > +{
> > > > + switch (su->type) {
> > > > + case AS3711_BL_SU1:
> > > > + return container_of(su, struct as3711_bl_supply, su1);
> > > > + case AS3711_BL_SU2:
> > > > + return container_of(su, struct as3711_bl_supply, su2);
> > > > + }
> > > > + return NULL;
> > > > +}
> > > > +
> > > > +static int as3711_set_brightness_auto_i(struct as3711_bl_data *data,
> > > > + unsigned int brightness)
> > > > +{
> > > > + struct as3711_bl_supply *supply = to_supply(data);
> > > > + struct as3711 *as3711 = supply->as3711;
> > > > + const struct as3711_bl_pdata *pdata = supply->pdata;
> > > > + int ret = 0;
> > > > +
> > > > + /* Only all equal current values are supported */
> > > > + if (pdata->su2_auto_curr1)
> > > > + ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE,
> > > > + brightness);
> > > > + if (!ret && pdata->su2_auto_curr2)
> > > > + ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE,
> > > > + brightness);
> > > > + if (!ret && pdata->su2_auto_curr3)
> > > > + ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE,
> > > > + brightness);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int as3711_set_brightness_v(struct as3711 *as3711,
> > > > + unsigned int brightness,
> > > > + unsigned int reg)
> > > > +{
> > > > + if (brightness > 31)
> > > > + return -EINVAL;
> > > > +
> > > > + return regmap_update_bits(as3711->regmap, reg, 0xf0,
> > > > + brightness << 4);
> > > > +}
> > > > +
> > > > +static int as3711_bl_su2_reset(struct as3711_bl_supply *supply)
> > > > +{
> > > > + struct as3711 *as3711 = supply->as3711;
> > > > + int ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_5,
> > > > + 3, supply->pdata->su2_fbprot);
> > > > + if (!ret)
> > > > + ret = regmap_update_bits(as3711->regmap,
> > > > + AS3711_STEPUP_CONTROL_2, 1, 0);
> > > > + if (!ret)
> > > > + ret = regmap_update_bits(as3711->regmap,
> > > > + AS3711_STEPUP_CONTROL_2, 1, 1);
> > > > + return ret;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Someone with less fragile or less expensive hardware could try to simplify
> > > > + * the brightness adjustment procedure.
> > > > + */
> > > > +static int as3711_bl_update_status(struct backlight_device *bl)
> > > > +{
> > > > + struct as3711_bl_data *data = bl_get_data(bl);
> > > > + struct as3711_bl_supply *supply = to_supply(data);
> > > > + struct as3711 *as3711 = supply->as3711;
> > > > + int brightness = bl->props.brightness;
> > > > + int ret = 0;
> > > > +
> > > > + dev_dbg(&bl->dev, "%s(): brightness %u, pwr %x, blank %x, state %x\n",
> > > > + __func__, bl->props.brightness, bl->props.power,
> > > > + bl->props.fb_blank, bl->props.state);
> > > > +
> > > > + if (bl->props.power != FB_BLANK_UNBLANK ||
> > > > + bl->props.fb_blank != FB_BLANK_UNBLANK ||
> > > > + bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> > > > + brightness = 0;
> > > > +
> > > > + if (data->type = AS3711_BL_SU1) {
> > > > + ret = as3711_set_brightness_v(as3711, brightness,
> > > > + AS3711_STEPUP_CONTROL_1);
> > > > + } else {
> > > > + const struct as3711_bl_pdata *pdata = supply->pdata;
> > > > +
> > > > + switch (pdata->su2_feedback) {
> > > > + case AS3711_SU2_VOLTAGE:
> > > > + ret = as3711_set_brightness_v(as3711, brightness,
> > > > + AS3711_STEPUP_CONTROL_2);
> > > > + break;
> > > > + case AS3711_SU2_CURR_AUTO:
> > > > + ret = as3711_set_brightness_auto_i(data, brightness / 4);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > + if (brightness) {
> > > > + ret = as3711_bl_su2_reset(supply);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > + udelay(500);
> > > > + ret = as3711_set_brightness_auto_i(data, brightness);
> > > > + } else {
> > > > + ret = regmap_update_bits(as3711->regmap,
> > > > + AS3711_STEPUP_CONTROL_2, 1, 0);
> > > > + }
> > > > + break;
> > > > + /* Manual one current feedback pin below */
> > > > + case AS3711_SU2_CURR1:
> > > > + ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE,
> > > > + brightness);
> > > > + break;
> > > > + case AS3711_SU2_CURR2:
> > > > + ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE,
> > > > + brightness);
> > > > + break;
> > > > + case AS3711_SU2_CURR3:
> > > > + ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE,
> > > > + brightness);
> > > > + break;
> > > > + default:
> > > > + ret = -EINVAL;
> > > > + }
> > > > + }
> > > > + if (!ret)
> > > > + data->brightness = brightness;
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int as3711_bl_get_brightness(struct backlight_device *bl)
> > > > +{
> > > > + struct as3711_bl_data *data = bl_get_data(bl);
> > > > +
> > > > + return data->brightness;
> > > > +}
> > > > +
> > > > +static const struct backlight_ops as3711_bl_ops = {
> > > > + .update_status = as3711_bl_update_status,
> > > > + .get_brightness = as3711_bl_get_brightness,
> > > > +};
> > > > +
> > > > +static int as3711_bl_init_su2(struct as3711_bl_supply *supply)
> > > > +{
> > > > + struct as3711 *as3711 = supply->as3711;
> > > > + const struct as3711_bl_pdata *pdata = supply->pdata;
> > > > + u8 ctl = 0;
> > > > + int ret;
> > > > +
> > > > + dev_dbg(as3711->dev, "%s(): use %u\n", __func__, pdata->su2_feedback);
> > > > +
> > > > + /* Turn SU2 off */
> > > > + ret = regmap_write(as3711->regmap, AS3711_STEPUP_CONTROL_2, 0);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > +
> > > > + switch (pdata->su2_feedback) {
> > > > + case AS3711_SU2_VOLTAGE:
> > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 0);
> > > > + break;
> > > > + case AS3711_SU2_CURR1:
> > > > + ctl = 1;
> > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 1);
> > > > + break;
> > > > + case AS3711_SU2_CURR2:
> > > > + ctl = 4;
> > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 2);
> > > > + break;
> > > > + case AS3711_SU2_CURR3:
> > > > + ctl = 0x10;
> > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 3);
> > > > + break;
> > > > + case AS3711_SU2_CURR_AUTO:
> > > > + if (pdata->su2_auto_curr1)
> > > > + ctl = 2;
> > > > + if (pdata->su2_auto_curr2)
> > > > + ctl |= 8;
> > > > + if (pdata->su2_auto_curr3)
> > > > + ctl |= 0x20;
> > > > + ret = 0;
> > > > + break;
> > > > + default:
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + if (!ret)
> > > > + ret = regmap_write(as3711->regmap, AS3711_CURR_CONTROL, ctl);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int as3711_bl_register(struct platform_device *pdev,
> > > > + unsigned int max_brightness, struct as3711_bl_data *su)
> > > > +{
> > > > + struct backlight_properties props = {.type = BACKLIGHT_RAW,};
> > > > + struct backlight_device *bl;
> > > > +
> > > > + /* max tuning I = 31uA for voltage- and 38250uA for current-feedback */
> > > > + props.max_brightness = max_brightness;
> > > > +
> > > > + bl = backlight_device_register(su->type = AS3711_BL_SU1 ?
> > > > + "as3711-su1" : "as3711-su2",
> > > > + &pdev->dev, su,
> > > > + &as3711_bl_ops, &props);
> > > > + if (IS_ERR(bl)) {
> > > > + dev_err(&pdev->dev, "failed to register backlight\n");
> > > > + return PTR_ERR(bl);
> > > > + }
> > > > +
> > > > + bl->props.brightness = props.max_brightness;
> > > > +
> > > > + backlight_update_status(bl);
> > > > +
> > > > + su->bl = bl;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int as3711_backlight_probe(struct platform_device *pdev)
> > > > +{
> > > > + struct as3711_bl_pdata *pdata = dev_get_platdata(&pdev->dev);
> > > > + struct as3711 *as3711 = dev_get_drvdata(pdev->dev.parent);
> > > > + struct as3711_bl_supply *supply;
> > > > + struct as3711_bl_data *su;
> > > > + unsigned int max_brightness;
> > > > + int ret;
> > > > +
> > > > + if (!pdata || (!pdata->su1_fb && !pdata->su2_fb)) {
> > > > + dev_err(&pdev->dev, "No platform data, exiting...\n");
> > > > + return -ENODEV;
> > > > + }
> > > > +
> > > > + /*
> > > > + * Due to possible hardware damage I chose to block all modes,
> > > > + * unsupported on my hardware. Anyone, wishing to use any of those modes
> > > > + * will have to first review the code, then activate and test it.
> > > > + */
> > > > + if (pdata->su1_fb ||
> > > > + pdata->su2_fbprot != AS3711_SU2_GPIO4 ||
> > > > + pdata->su2_feedback != AS3711_SU2_CURR_AUTO) {
> > > > + dev_warn(&pdev->dev,
> > > > + "Attention! An untested mode has been chosen!\n"
> > > > + "Please, review the code, enable, test, and report success:-)\n");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + supply = devm_kzalloc(&pdev->dev, sizeof(*supply), GFP_KERNEL);
> > > > + if (!supply)
> > > > + return -ENOMEM;
> > > > +
> > > > + supply->as3711 = as3711;
> > > > + supply->pdata = pdata;
> > > > +
> > > > + if (pdata->su1_fb) {
> > > > + su = &supply->su1;
> > > > + su->fb_name = pdata->su1_fb;
> > > > + su->type = AS3711_BL_SU1;
> > > > +
> > > > + max_brightness = min(pdata->su1_max_uA, 31);
> > > > + ret = as3711_bl_register(pdev, max_brightness, su);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > + }
> > > > +
> > > > + if (pdata->su2_fb) {
> > > > + su = &supply->su2;
> > > > + su->fb_name = pdata->su2_fb;
> > > > + su->type = AS3711_BL_SU2;
> > > > +
> > > > + switch (pdata->su2_fbprot) {
> > > > + case AS3711_SU2_GPIO2:
> > > > + case AS3711_SU2_GPIO3:
> > > > + case AS3711_SU2_GPIO4:
> > > > + case AS3711_SU2_LX_SD4:
> > > > + break;
> > > > + default:
> > > > + ret = -EINVAL;
> > > > + goto esu2;
> > > > + }
> > > > +
> > > > + switch (pdata->su2_feedback) {
> > > > + case AS3711_SU2_VOLTAGE:
> > > > + max_brightness = min(pdata->su2_max_uA, 31);
> > > > + break;
> > > > + case AS3711_SU2_CURR1:
> > > > + case AS3711_SU2_CURR2:
> > > > + case AS3711_SU2_CURR3:
> > > > + case AS3711_SU2_CURR_AUTO:
> > > > + max_brightness = min(pdata->su2_max_uA / 150, 255);
> > > > + break;
> > > > + default:
> > > > + ret = -EINVAL;
> > > > + goto esu2;
> > > > + }
> > > > +
> > > > + ret = as3711_bl_init_su2(supply);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > +
> > > > + ret = as3711_bl_register(pdev, max_brightness, su);
> > > > + if (ret < 0)
> > > > + goto esu2;
> > > > + }
> > > > +
> > > > + platform_set_drvdata(pdev, supply);
> > > > +
> > > > + return 0;
> > > > +
> > > > +esu2:
> > > > + backlight_device_unregister(supply->su1.bl);
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int as3711_backlight_remove(struct platform_device *pdev)
> > > > +{
> > > > + struct as3711_bl_supply *supply = platform_get_drvdata(pdev);
> > > > +
> > > > + backlight_device_unregister(supply->su1.bl);
> > > > + backlight_device_unregister(supply->su2.bl);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static struct platform_driver as3711_backlight_driver = {
> > > > + .driver = {
> > > > + .name = "as3711-backlight",
> > > > + .owner = THIS_MODULE,
> > > > + },
> > > > + .probe = as3711_backlight_probe,
> > > > + .remove = as3711_backlight_remove,
> > > > +};
> > > > +
> > > > +module_platform_driver(as3711_backlight_driver);
> > > > +
> > > > +MODULE_DESCRIPTION("Backlight Driver for AS3711 PMICs");
> > > > +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de");
> > > > +MODULE_LICENSE("GPL");
> > > > +MODULE_ALIAS("platform:as3711-backlight");
> > > > --
> > > > 1.7.2.5
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
> >
> > ---
> > Guennadi Liakhovetski, Ph.D.
> > Freelance Open-Source Software Developer
> > http://www.open-technology.de/
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Alexander Holler @ 2013-02-05 7:08 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, linux-kernel, linux-fbdev,
Florian Tobias Schandinat, Bernie Thompson, Steve Glendinning,
stable
In-Reply-To: <20130204192514.GA32318@kroah.com>
Am 04.02.2013 20:25, schrieb Greg KH:
> On Mon, Feb 04, 2013 at 08:17:04PM +0100, Alexander Holler wrote:
>> Am 04.02.2013 13:05, schrieb Alexander Holler:
>>> Am 04.02.2013 02:14, schrieb Greg KH:
>>>
>>>> So you are right in that your driver will wait for forever for a
>>>> disconnect() to happen, as it will never be called. I don't understand
>>>> the problem that this is causing when it happens. What's wrong with
>>>> udlfb that having the cpu suddently reset as the powerdown happened
>>>> without it knowing about it?
>>>
>>> There is nothing wrong with that. I've just explained why a problem
>>> doesn't occur on shutdown but on disconnect (of the device).
>>
>> Maybe my explanation before was just to long and I try to explain it
>> a bit shorter:
>>
>> If a device gets disconnected, the disconnect in udlfb might wait
>> forever in down_interruptible() (because it waits for an urb it
>> never receives). This even prevents a shutdown afterwards, because
>> that down_interruptible() never receives a signal (at shutdown,
>> because kernel threads don't get such).
>
> Where was that urb when the disconnect happened? The USB core should
> call your urb callback for any outstanding urbs at that point in time,
> with the proper error flag being set, are you handling that properly?
I don't know where that urb is as I don't handle it. I just know that
_interruptible doesn't make any sense and _timeout is necessary here.
But as nobody else seems to have a problem, nobody else see seems to see
the problem there and I seem to be unable to explain it, just ignore
that patch.
Regards,
Alexander
^ permalink raw reply
* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Greg KH @ 2013-02-04 19:25 UTC (permalink / raw)
To: Alexander Holler
Cc: Andrew Morton, linux-kernel, linux-fbdev,
Florian Tobias Schandinat, Bernie Thompson, Steve Glendinning,
stable
In-Reply-To: <51100930.6080405@ahsoftware.de>
On Mon, Feb 04, 2013 at 08:17:04PM +0100, Alexander Holler wrote:
> Am 04.02.2013 13:05, schrieb Alexander Holler:
> >Am 04.02.2013 02:14, schrieb Greg KH:
> >
> >>So you are right in that your driver will wait for forever for a
> >>disconnect() to happen, as it will never be called. I don't understand
> >>the problem that this is causing when it happens. What's wrong with
> >>udlfb that having the cpu suddently reset as the powerdown happened
> >>without it knowing about it?
> >
> >There is nothing wrong with that. I've just explained why a problem
> >doesn't occur on shutdown but on disconnect (of the device).
>
> Maybe my explanation before was just to long and I try to explain it
> a bit shorter:
>
> If a device gets disconnected, the disconnect in udlfb might wait
> forever in down_interruptible() (because it waits for an urb it
> never receives). This even prevents a shutdown afterwards, because
> that down_interruptible() never receives a signal (at shutdown,
> because kernel threads don't get such).
Where was that urb when the disconnect happened? The USB core should
call your urb callback for any outstanding urbs at that point in time,
with the proper error flag being set, are you handling that properly?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Alexander Holler @ 2013-02-04 19:17 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, linux-kernel, linux-fbdev,
Florian Tobias Schandinat, Bernie Thompson, Steve Glendinning,
stable
In-Reply-To: <510FA409.2080201@ahsoftware.de>
Am 04.02.2013 13:05, schrieb Alexander Holler:
> Am 04.02.2013 02:14, schrieb Greg KH:
>
>> So you are right in that your driver will wait for forever for a
>> disconnect() to happen, as it will never be called. I don't understand
>> the problem that this is causing when it happens. What's wrong with
>> udlfb that having the cpu suddently reset as the powerdown happened
>> without it knowing about it?
>
> There is nothing wrong with that. I've just explained why a problem
> doesn't occur on shutdown but on disconnect (of the device).
Maybe my explanation before was just to long and I try to explain it a
bit shorter:
If a device gets disconnected, the disconnect in udlfb might wait
forever in down_interruptible() (because it waits for an urb it never
receives). This even prevents a shutdown afterwards, because that
down_interruptible() never receives a signal (at shutdown, because
kernel threads don't get such).
So the change from down_timeout() to down_interruptible() in
dlfb_free_urb_list() with commit
33077b8d3042e01da61924973e372abe589ba297 only results in that the
following code (thus the break there) will never be reached if an urb
got missed (because of a disconnect).
And the accompanying comment (... at shutdown) is misleading, because on
shutdown, the kernel thread which calls dlfb_free_urb_list() never gets
a signal, so the interruption just never happens.
As I've experienced the "missing urb on disconnect" problem quiet often,
I've changed that down_interruptible() to down_timeout() (in v1 and in
v2 to down_timeout_interruptible, because I wasn't aware that no signal
arrives on shutdown).
Hmm, ok, that explanation isn't much shorter. ;)
Regards,
Alexander
^ permalink raw reply
* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Alexander Holler @ 2013-02-04 12:05 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, linux-kernel, linux-fbdev,
Florian Tobias Schandinat, Bernie Thompson, Steve Glendinning,
stable
In-Reply-To: <20130204011413.GA6413@kroah.com>
Am 04.02.2013 02:14, schrieb Greg KH:
> So you are right in that your driver will wait for forever for a
> disconnect() to happen, as it will never be called. I don't understand
> the problem that this is causing when it happens. What's wrong with
> udlfb that having the cpu suddently reset as the powerdown happened
> without it knowing about it?
There is nothing wrong with that. I've just explained why a problem
doesn't occur on shutdown but on disconnect (of the device).
Regards,
Alexander
^ 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