[11/11] ARM: versatile: move CLCD configuration to device tree

Message ID 1454594660-7532-12-git-send-email-linus.walleij@linaro.org
State New
Headers show

Commit Message

Linus Walleij Feb. 4, 2016, 2:04 p.m.
This moves the versatile CLCD configuration to the device tree by:

- Deleting the board file set-up of CLCD displays and quirks,
  instead relying on the driver to handle this. The driver will
  attempt to auto-detect (like the board file did) and match to
  a corresponding panel in the device tree.

- Defining all auto-detectable panels in the device tree for the
  versatile-ab, defaulting the first one to VGA. The right
  panel will be selected at panel initialization, and should
  just work for the IB1 daughterboard panels, like EPSON.

- Creating a special superset DTS file for the IB2 daughterboard
  (phone form-factor) equipped Versatile, overriding the default VGA
  display with the Sanyo 2.5" portrait display definitions, so that
  the IB2-equipped Versatile can be used with this. This follows
  the pattern of how we define the Versatile PB as a superset of
  Versatile AB.

Tested on Versatile AB with just VGA with the default device tree,
and with the IB2 daughterboard with the custom IB2 device tree.
Tested to shunt in XVGA by modifying the device tree and this works
too. Also tested on QEMU for Versatile in both VGA and Sanyo 2.5"
mode. I don't have the IB1 daughterboard and its add-on displays,
but it should work as long as the detection mechanism and device
tree parameters are sound.

Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 arch/arm/boot/dts/Makefile             |   1 +
 arch/arm/boot/dts/versatile-ab-ib2.dts |  20 ++++
 arch/arm/boot/dts/versatile-ab.dts     | 203 ++++++++++++++++++++++++++++++++-
 arch/arm/mach-versatile/versatile_dt.c | 162 --------------------------
 4 files changed, 220 insertions(+), 166 deletions(-)
 create mode 100644 arch/arm/boot/dts/versatile-ab-ib2.dts

-- 
2.4.3

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

Comments

Linus Walleij Feb. 17, 2016, 4:17 p.m. | #1
On Wed, Feb 17, 2016 at 10:09 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

>> Tested on Versatile AB with just VGA with the default device tree,

>> and with the IB2 daughterboard with the custom IB2 device tree.

>> Tested to shunt in XVGA by modifying the device tree and this works

>> too. Also tested on QEMU for Versatile in both VGA and Sanyo 2.5"

>> mode. I don't have the IB1 daughterboard and its add-on displays,

>> but it should work as long as the detection mechanism and device

>> tree parameters are sound.

>

> Well... I don't like this very much. The .dts should contain

> descriptions for hardware that is connected. Have you looked at DT

> overlays? I think they would be a much better match for this.


I'll look into it. I don't know how good those are. They must be overlaid
at runtime and anyways exist somewhere in memory so they can be
overlaid in there.

> What's the SYS_CLCD register? An EEPROM or such, programmed when the

> board is manufactured? Is the panel meant to be switchable by the user,

> possibly to a panel that's not "standard"?


As Russell points out: it's a register that contains a number saying what
panel is connected.

So it is plug-n-play and I want to preserve this in the patch.

The alternative is to make one DTS per display type connected, but that is
loosing all the nice plug'n'play :(

But if an overlay can do the same, I'm game for it.

Yours,
Linus Walleij
--
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
Linus Walleij Feb. 18, 2016, 8:31 p.m. | #2
On Thu, Feb 18, 2016 at 2:37 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 18/02/16 15:12, Russell King - ARM Linux wrote:

>> On Thu, Feb 18, 2016 at 01:52:32PM +0200, Tomi Valkeinen wrote:

>>> In my opinion the best option would be to use DT overlays, but so that

>>> the bootloader would supply them, or construct the dtb. But afaik that's

>>> not possible at the moment. And perhaps I think that's the best option

>>> only because I don't work with the bootloaders =).

>>>

>>> So, I don't like this, but I don't have a good suggestion how to do it

>>> better with the infrastructure in place at the moment.

>>

>> The danger of that position is that we end up with nothing happening,

>> and the problem remaining unresolved, which then pushes people into

>> maintaining patches out of mainline just to have a working setup -

>> which then pushes people to vendor trees.

>

> I agree.

>

> I didn't express my position clearly: I'm ok with the approach, if we

> don't come up with anything better.


I'm checking to see if I can use the overlay primitives directly from
the kernel.

It seems possible, basically to have a panel defined in the device tree
for VGA as default, and then augment it depending on what we detect.
Meaning I start to overwrite clock-frequency, pixelclk-active, hactive etc.
It basically means we go back to having a database in the kernel.

I would then even overwrite the .compatible string from the kernel.

It may be more elegant than this solution though?

> But if we go with this approach, it must be understood that it may cause

> problems later. It's not the most maintainable approach.

>

> I'd also like to have an ack from the DT maintainers, as I think this is

> somewhat of an abuse of the DT.


What I am doing in this patch is sort of a "Schrödinger's cat approach",
I basically say that the cat is both dead and alive until we open the
box, i.e. all displays are connected until we figure out which one it
actually is.

Basically the approach taken could even handle the case of switching
a display at runtime. Though I don't think the hardware would like that.

Yours,
Linus Walleij
--
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
Linus Walleij Feb. 21, 2016, 10:39 p.m. | #3
On Thu, Feb 18, 2016 at 12:52 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> For panels we need DT fragments. The question is where these fragments

> are and, possibly, who who loads them.


I hacked something up that augments the device tree from the kernel,
given you have a node with all the props you want to augment, tell me
what you think of this and whether I should continue in this direction...
also the DT people need to be involved:

#include "../../drivers/of/of_private.h"
/* Obviously make the property duplication function public instead,
it's a test */

struct versatile_panel {
    u32 id;
    char *compatible;
    u32 clock_frequency;
    u32 pixelclk_active;
    u32 hsync_active;
    u32 vsync_active;
    u32 de_active;
    u32 hactive;
    u32 hback_porch;
    u32 hfront_porch;
    u32 hsync_len;
    u32 vactive;
    u32 vback_porch;
    u32 vfront_porch;
    u32 vsync_len;
    bool ib2;
};

static const struct versatile_panel versatile_panels[] = {
    {
        .id = SYS_CLCD_ID_VGA,
        .compatible = "VGA",
        .clock_frequency = 25175000,
        .pixelclk_active = 0,
        .hsync_active = 1,
        .vsync_active = 1,
        .de_active = 1,
        .hactive = 640,
        .hback_porch = 48,
        .hfront_porch = 16,
        .hsync_len = 96,
        .vactive = 480,
        .vback_porch = 33,
        .vfront_porch = 10,
        .vsync_len = 2,
    },
    {
        .id = SYS_CLCD_ID_SANYO_3_8,
        .compatible = "sanyo,tm38qv67a02a",
        .clock_frequency = 10000000,
        .pixelclk_active = 1,
        .hsync_active = 1,
        .vsync_active = 1,
        .de_active = 1,
        .hactive = 320,
        .hback_porch = 6,
        .hfront_porch = 6,
        .hsync_len = 6,
        .vactive = 240,
        .vback_porch = 6,
        .vfront_porch = 6,
        .vsync_len = 0,
    },
    {
        .id = SYS_CLCD_ID_SHARP_8_4,
        .compatible = "sharp,lq084v1dg21",
        .clock_frequency = 25175000,
        .pixelclk_active = 0,
        .hsync_active = 1,
        .vsync_active = 1,
        .de_active = 1,
        .hactive = 640,
        .hback_porch = 48,
        .hfront_porch = 16,
        .hsync_len = 96,
        .vactive = 480,
        .vback_porch = 33,
        .vfront_porch = 10,
        .vsync_len = 2,
    },
    {
        .id = SYS_CLCD_ID_EPSON_2_2,
        .compatible = "epson,l2f50113t00",
        .clock_frequency = 16000000,
        .pixelclk_active = 0,
        .hsync_active = 1,
        .vsync_active = 1,
        .de_active = 1,
        .hactive = 176,
        .hback_porch = 3,
        .hfront_porch = 2,
        .hsync_len = 3,
        .vactive = 220,
        .vback_porch = 1,
        .vfront_porch = 0,
        .vsync_len = 2,
    },
    {
        .id = SYS_CLCD_ID_SANYO_2_5,
        .compatible = "sanyo,alr252rgt",
        .ib2 = true,
        .clock_frequency = 5440000,
        .pixelclk_active = 0,
        .hsync_active = 0,
        .vsync_active = 0,
        .de_active = 1,
        .hactive = 240,
        .hback_porch = 20,
        .hfront_porch = 10,
        .hsync_len = 10,
        .vactive = 320,
        .vback_porch = 2,
        .vfront_porch = 2,
        .vsync_len = 2,
    },
};

static void update_timings_prop(struct device *dev,
                struct of_changeset *cset,
                struct device_node *timings,
                const char *propname,
                u32 val)
{
    struct property *prop, *new;
    __be32 *dt_val;

    prop = of_find_property(timings, propname, NULL);
    if (!prop) {
        dev_err(dev, "could not find property \"%s\" - skipping\n",
            propname);
        return;
    }
    new = __of_prop_dup(prop, GFP_KERNEL);
    if (!new) {
        dev_err(dev, "could not copy property \"%s\" - skipping\n",
            propname);
        return;
    }

    dt_val = new->value;
    *dt_val = cpu_to_be32(val);

    of_changeset_update_property(cset, timings, new);
}

static int versatile_overwrite_of_panel(struct device *dev,
                    struct device_node *panel,
                    const struct versatile_panel *vpanel)
{
    struct of_changeset cset;
    struct device_node *timings;
    int ret;

    dev_info(dev, "CLCD: overwriting device tree\n");

    of_changeset_init(&cset);
    /* Find the timings node */
    timings = of_get_child_by_name(panel, "panel-timing");
    if (!timings) {
        dev_err(dev, "could not find panel timing node\n");
        goto err_destroy_cs;
    }
    update_timings_prop(dev, &cset, timings, "clock-frequency",
                vpanel->clock_frequency);
    update_timings_prop(dev, &cset, timings, "pixelclk-active",
                vpanel->pixelclk_active);
    update_timings_prop(dev, &cset, timings, "hsync-active",
                vpanel->hsync_active);
    update_timings_prop(dev, &cset, timings, "vsync-active",
                vpanel->vsync_active);
    update_timings_prop(dev, &cset, timings, "de-active",
                vpanel->de_active);
    update_timings_prop(dev, &cset, timings, "hactive",
                vpanel->hactive);
    update_timings_prop(dev, &cset, timings, "hback-porch",
                vpanel->hback_porch);
    update_timings_prop(dev, &cset, timings, "hsync-len",
                vpanel->hsync_len);
    update_timings_prop(dev, &cset, timings, "vactive",
                vpanel->vactive);
    update_timings_prop(dev, &cset, timings, "vback-porch",
                vpanel->vback_porch);
    update_timings_prop(dev, &cset, timings, "vfront-porch",
                vpanel->vfront_porch);
    update_timings_prop(dev, &cset, timings, "vsync-len",
                vpanel->hsync_len);

    ret = of_changeset_apply(&cset);
    if (ret) {
        dev_err(dev, "could not apply device tree changeset\n");
        goto err_destroy_cs;
    }
    return ret;

err_destroy_cs:
    of_changeset_destroy(&cset);
    return ret;
}

static void versatile_panel_probe(struct device *dev,
                  struct device_node *endpoint)
{
    struct versatile_panel const *vpanel = NULL;
    struct device_node *panel = NULL;
    u32 val;
    int ret;
    int i;

    /*
     * The Versatile CLCD has a panel auto-detection mechanism.
     * We use this and look for the compatible panel in the
     * device tree.
     */
    ret = regmap_read(versatile_syscon_map, SYS_CLCD, &val);
    if (ret) {
        dev_err(dev, "cannot read CLCD syscon register\n");
        return;
    }
    val &= SYS_CLCD_CLCDID_MASK;
    dev_info(dev, "SYS_CLCD=%08x\n", val);

    /* First find corresponding panel information */
    for (i = 0; i < ARRAY_SIZE(versatile_panels); i++) {
        vpanel = &versatile_panels[i];

        if (val == vpanel->id) {
            dev_err(dev, "autodetected panel \"%s\"\n",
                vpanel->compatible);
            break;
        }
    }
    if (i == ARRAY_SIZE(versatile_panels)) {
        dev_err(dev, "could not auto-detect panel\n");
        return;
    }

    /* This is the default panel node in the device tree */
    panel = of_graph_get_remote_port_parent(endpoint);
    if (!panel) {
        dev_err(dev, "could not locate remote port for panel\n");
        return;
    }

    /*
     * So now we dynamically update the display properties of the
     * panel in accordance to what was detected..
     */
    ret = versatile_overwrite_of_panel(dev, panel, vpanel);
    if (ret) {
        dev_err(dev, "cannot overwrite devicetree panel\n");
        return;
    }

    /*
     * If we have a Sanyo 2.5" port
     * that we're running on an IB2 and proceed to look for the
     * IB2 syscon regmap.
     */
    if (!vpanel->ib2)
        return;

    versatile_ib2_map = syscon_regmap_lookup_by_compatible(
        "arm,versatile-ib2-syscon");
    if (IS_ERR(versatile_ib2_map)) {
        dev_err(dev, "could not locate IB2 control register\n");
        versatile_ib2_map = NULL;
        return;
    }
}


This actually works. But would need some public device tree
property augmentation API instead of the hacks. Should I pursue
it?

Yours,
Linus Walleij
--
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
Linus Walleij Feb. 22, 2016, 3:54 p.m. | #4
On Mon, Feb 22, 2016 at 4:41 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> After thinking this a bit and discussing it with Laurent P., generally

> speaking I still think that the only sane option is that the bootloader

> does any detection needed and provides the kernel a .dtb that contains

> the HW that is connected. No board specific drivers are needed on the

> kernel side.

>

> In some cases userspace loaded DT overlays may be fine, if the userspace

> can do the detection and the device in question is not somehow critical

> to operation. But I think displays are critical, and afaik in Versatile

> case the userspace can't even do the detection (?).

>

> The third option is to have board specific display handling code and the

> display HW data in the kernel, as you've done in the patches.


Yeah correct...

> But, of course, which option should be used for which board is not

> always clear...

>

> What bootloader is used on Versatile?


It's U-boot indeed. Not that I've tried to compile or use it, I got it
as binary from ARM.

> If it's some proprietary loader

> which can't be changed, then the bootloader option is out, and I guess

> it points to the third option, i.e. either the version in this patch or

> the earlier version. If it's u-boot, I would suggest going for the

> bootloader option.

>

> Afaik u-boot doesn't support combining DT fragments yet. But (also

> afaik) the u-boot maintainer is ok with the idea. And I know there are

> others (for example TI) interested in the same functionality.


Hm OK.... so the bootloader need to be better at augmenting device
trees than the kernel. Well... The problem is that there are a bunch
of deployed systems out there and they all need to have their boot loader
updated then, which may be OK since it's ARM development boards
but I don't know.

> Now, adding that support might take some time, and in the meantime it'd

> be good to get the HW working with kernel with a temporary solution. To

> do that, my suggestion is basically "any solution which requires no

> (temporary) changes to .dts".

>

> While I don't like too much the solution in the patch here, it's all

> inside kernel code and can be dropped easily, right? If we would merge

> the the multi-endpoint solution you had in the earlier patch, you would

> have to support that .dts in the future too.


To go with this solution I need to extend the drivers/of library to be
able to update properties properly instead of this hack, and that is
non-reversible if we start to use it.

It is not really an overlay because the DT stuff is dynamically
augmented by the kernel, not taken from somewhere else.

Yours,
Linus Walleij
--
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
Linus Walleij Feb. 23, 2016, 9:08 a.m. | #5
On Thu, Feb 4, 2016 at 3:04 PM, Linus Walleij <linus.walleij@linaro.org> wrote:

> This moves the versatile CLCD configuration to the device tree by:


As this Schrödinger's cat approach seems controversial, as well
as the alternative to manipulate the DT in memory at run-time,
I will respin the series without the full Versatile support, adding
plug-in for the other ARM boards and and half-baked support
for the Versatile supporting just VGA (like the other reference
designs from ARM).

The pluggable displays prove yet again to be a problem to the
world, sigh.

I will think of a better solution, if any, for this for v4.6, but will
put forward something that handles the Nomadik and all the
other ARM reference designs for now.

Yours,
Linus Walleij
--
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
Linus Walleij Feb. 23, 2016, 10:10 a.m. | #6
On Tue, Feb 23, 2016 at 10:34 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 23 February 2016 10:08:05 Linus Walleij wrote:

>> I will think of a better solution, if any, for this for v4.6, but will

>> put forward something that handles the Nomadik and all the

>> other ARM reference designs for now.

>>

>

> How about still describing all known panels in the DT marked 'status="disabled"',

> but then having a minimal piece of board specific code that just enables

> whichever one gets detected at boot time?


Close but no cigar :D

It means that all panels (enabled and all disabled) have to have the same
endpoint in the of_graph.

I tried it:

    panel@0 {
        compatible = "panel-dpi";
        port {
            clcd_panel: endpoint {
                remote-endpoint = <&clcd_pads>;
            };
        };
(...)

    panel@1 {
        compatible = "panel-dpi";
        port {
            clcd_panel: endpoint {
                remote-endpoint = <&clcd_pads>;
            };
        };
(...)


        display@10120000 {
(...)
            port {
(...)
                clcd_pads: endpoint@0 {
                    reg = <0>;
                    remote-endpoint = <&clcd_panel>;
                    arm,pl11x,tft-r0g0b0-pads = <1 7 13>;
                };
            };



Building the device tree:

ERROR (duplicate_label): ERROR (duplicate_label): Duplicate label
'clcd_panel' on /panel@1/port/endpoint and /panel@0/port/endpoint
Duplicate label 'clcd_panel' on /panel@1/port/endpoint and
/panel@0/port/endpoint

So the dtc does not allow duplicate node labels for the phandle
even if the node it resides in is disabled. Tough luck...

Yours,
Linus Walleij
--
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
Peter Maydell Feb. 23, 2016, 11:56 a.m. | #7
On 23 February 2016 at 09:58, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> I'm not quite sure how it works if, as in versatile display case, there

> are multiple DT overlays of which one has to be enabled. I hope there's

> support to choose which one to use via kernel cmdline or similar.

>

> I would personally like it much more if the bootloader would either

> compose a final dtb from DT fragments and pass it to the kernel, or

> alternatively the bootloader would pass the base dtb image and a bunch

> of DT overlays to the kernel, and the kernel would deal with the DT

> overlays.


Speaking as somebody who's written the "bootloader" code that's
used for what I guess are the majority of versatile kernel boots,
i.e. the one in QEMU, I think that requiring the bootloader to do this
would be a significant worsening from the current state.

Right now the bootloader doesn't need to do much at all with device
trees, except pass the kernel the DT that the user gave us, which
is just the kernel's own data structures in a separate file for
convenience. You need to do some very minor tweaks to the /chosen
node, but these can be handled the same way for any board and aren't
hardware specific. There's no need to worry about dt fragments
either for the bootloader or for the user. Imposing a new requirement
for the bootloader to have to probe hardware which it otherwise
has no need to even care about, and then edit and update the DT
in a board-specific manner, or have board-specific DT fragments,
seems like a totally unnecessary imposition on both bootloader
authors and end-users, and of course it would break booting newer
kernels on the great mass of already existing boot loaders and
QEMU installs.

The kernel is in a position to probe the display hardware and determine
what is there, and do the right thing, and that's exactly what it
does today. The kernel should continue to do this.
The advantage of DT is that it allows moving information about
non-probeable hardware that was previously hardwired in the kernel
C sources into a separate data structure, but the versatile displays
are not non-probeable. I can see no benefit at all from hardwiring into
the dt something which the kernel has previously been successfully
dynamically getting right without any bootloader intervention -- it just
makes the kernel less flexible and less user-friendly.

thanks
-- PMM
--
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
Linus Walleij Feb. 23, 2016, 1:08 p.m. | #8
On Tue, Feb 23, 2016 at 12:56 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 23 February 2016 at 09:58, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

>> I'm not quite sure how it works if, as in versatile display case, there

>> are multiple DT overlays of which one has to be enabled. I hope there's

>> support to choose which one to use via kernel cmdline or similar.

>>

>> I would personally like it much more if the bootloader would either

>> compose a final dtb from DT fragments and pass it to the kernel, or

>> alternatively the bootloader would pass the base dtb image and a bunch

>> of DT overlays to the kernel, and the kernel would deal with the DT

>> overlays.

>

> Speaking as somebody who's written the "bootloader" code that's

> used for what I guess are the majority of versatile kernel boots,

> i.e. the one in QEMU, I think that requiring the bootloader to do this

> would be a significant worsening from the current state.

>

> Right now the bootloader doesn't need to do much at all with device

> trees, except pass the kernel the DT that the user gave us, which

> is just the kernel's own data structures in a separate file for

> convenience. You need to do some very minor tweaks to the /chosen

> node, but these can be handled the same way for any board and aren't

> hardware specific. There's no need to worry about dt fragments

> either for the bootloader or for the user. Imposing a new requirement

> for the bootloader to have to probe hardware which it otherwise

> has no need to even care about, and then edit and update the DT

> in a board-specific manner, or have board-specific DT fragments,

> seems like a totally unnecessary imposition on both bootloader

> authors and end-users, and of course it would break booting newer

> kernels on the great mass of already existing boot loaders and

> QEMU installs.

>

> The kernel is in a position to probe the display hardware and determine

> what is there, and do the right thing, and that's exactly what it

> does today. The kernel should continue to do this.

> The advantage of DT is that it allows moving information about

> non-probeable hardware that was previously hardwired in the kernel

> C sources into a separate data structure, but the versatile displays

> are not non-probeable. I can see no benefit at all from hardwiring into

> the dt something which the kernel has previously been successfully

> dynamically getting right without any bootloader intervention -- it just

> makes the kernel less flexible and less user-friendly.


I agree with Peter and Russell on this, as you could guess.
Today the kernel knows about all the hardware and it
JustWorks(TM) and that is so neat.

I'm still trying to appeal to the subsystem maintainer as well.
Pretty tough deal.

Yours,
Linus Walleij
--
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
Linus Walleij Feb. 23, 2016, 1:16 p.m. | #9
On Tue, Feb 23, 2016 at 2:00 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> That would be almost the same as what's already in this patch, except

> (if I'm not mistaken) the detection part could be in platform code, and

> the fbdev driver itself would know nothing about board specific

> detection or board specific panel lists.


In this patch set the board/platform-specific plugins are separated
out adding the opaque .board_init() and .panel_init() to the driver.
The platform-specific code is in completely separate files this way,
and the CLCD driver itself just handles various versions of that
IP block.

> So maybe that would be a bit cleaner. Still ugly, I think =). I really

> don't like having possible-panels in the Schrödinger's DT data

> (http://www.angryflower.com/387.html).


OK I will focus my work on the DT-augment code instead.

> That said, maybe this is the best way to deal with Versatile, without

> requiring any change to the bootloader or the boot mechanism.


Depends on if the OF core maintainers will accept my patches to
dynamically alter DT properties at runtime. If I can't do that then
I have to go back to the Schrödinger patch.

> What is the current status of Versatile? Have we had working display

> with Versatile when booting with device tree? Or has the display been

> supported only with legacy boot?


Versatile is DT only as of kernel v4.5. The DT boot uses AUXDATA
which is the Frankenstein solution, bolting on a boardfile piece
to the DT boot and ignoring the existing panel bindings, and of
course standing in the way of cleaning things up.

IMO Schrödinger > Frankenstein.

Yours,
Linus Walleij
--
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
Peter Maydell Feb. 23, 2016, 1:49 p.m. | #10
On 23 February 2016 at 12:45, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> So, true, there's probing going on, but it's all board specific,

> requiring a board specific driver to support it in the kernel. And I

> think that makes the bootloader much better place for supporting it.


This doesn't seem to me like a reason to put the requirement
in the bootloader. A huge part of the purpose of the kernel
is to support the hardware (whether that's completely generic
and probeable, like PCI, or generic but not probeable, or
completely specific to a particular board). The kernel has to
support the hardware, and just because it happens to be board
specific hardware rather than generic hardware doesn't seem to
me to imply that the kernel gets to drop part of its core purpose.

> I think one of the core questions here is: do we want to start adding

> board specific drivers to the kernel, instead of dealing with it in the

> bootloader when possible? My understanding is that we've been trying to

> reduce board specific code from the kernel.


I think there's a difference between "reduce board specific code
in the kernel by replacing it with the combination of generic
or parameterisable code in the kernel plus a kernel data structure
(DT) that supplies the parameterisation needed", and "reduce
board specific code in the kernel by forcing the bootloader to
do the kernel's job for it".

thanks
-- PMM
--
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
Linus Walleij Feb. 25, 2016, 1:43 p.m. | #11
On Wed, Feb 24, 2016 at 1:13 PM, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:

> IMHO DT+overlays handle all your cases just fine.

>

> As far as I see these are the cases which we need to handle:

>

> 1) The expansion board in question has some means of identification, whether it’s an

> EEPROM or a GPIO keying combination etc. In that case it is the kernel’s job to match this

> id value with a dtbo firmware file and apply it. The blob is located via means of request_firmware().


Since the dawn of time the x86 people used that console to display
the early boot crawl and collect crash data. What you're suggesting
is that we can't get the console up until after the filesystems and mounts
are up so the kernel can read firmware files.

This kills of early boot graphics and getting crash logs on the fbdev
console until that has happened.

It also means there is no way to get the console up without the right
firmware files in the filesystem. I think that is really crap compared
to what we have today where the display will always come up, and
basically a regression.

I understand the stance with respect to things like add-on hardware
like a Bluetooth board or WLAN or whatnot. But the fbdev console
is just too basic, like a serial port IMO.

Sure in the ARM world we usually have a serial console, but this is
seriously breaking current practice.

Yours,
Linus Walleij
--
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
Linus Walleij Feb. 25, 2016, 2:04 p.m. | #12
On Tue, Feb 23, 2016 at 2:38 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> Maybe Versatile is different. If CLCD is only used on that board, or a

> small family of boards, from one vendor, I guess it is maintainable to

> have board specific driver parts for CLCD. But if CLCD can be used by

> many vendors in many different boards, I'd steer clear of board specific

> driver code.


OK I think at this point we would say that CLCD is a legacy driver.
It is a PrimceCell (IP block) made by ARM Ltd for their reference
designs, and intended for demonstration purposes. It is used in
these:
arch/arm/mach-integrator/
arch/arm/mach-versatile/
arch/arm/mach-realview/
arch/arm/mach-vexpress/

At the last iteration of their reference designs, ARM invented a
new display driver called HDLCD, which you can find in
drivers/gpu/drm/arm in linux-next. Thus the CLCD is now legacy.

As part of signing a deal with ARM to synthesize their silicon,
vendors get copies of the IP blocks to jumpstart design work.
Sometimes they will design their own display controller, sometimes
they will take the ARM CLCD and synthesize it right off
and not innovate around it at all. That is why CLCD also
appears in:
arch/arm/configs/axm55xx_defconfig
arch/arm/configs/lpc18xx_defconfig
arch/arm/boot/dts/lpc18xx.dtsi
arch/arm/configs/lpc32xx_defconfig
arch/arm/boot/dts/lpc32xx.dtsi
arch/arm/configs/netx_defconfig
arch/arm/configs/spear3xx_defconfig

Sometimes the vendors will tweak the CLCD. St Microelectronics
did the latter, and that is why I add support for that variant as
well.

HOWEVER: the ARM Versatile is the *only* platform I have
seen of these that have plug'n'play for the display.

*All* the others
will be very happy with *ONE* display defined as panel in the
device tree, and off they go. Usually VGA. And that will look
much like arch/arm/boot/dts/vexpress-v2m.dtsi already look
like today, using "panel-dpi" to define their displays.

They and their displays may need some board-specific or SoC
specific tweaks though, just like the Nomadik. The Vexpress is
happy to be able to go without, because I guess it is hard-coded
to just use the DVI output, so no path for the signal needs to be
set up.

I add support for doing this for the Integrator and RealView in
the patch set, by grabbing a handle to the system controller
where they have a few "misc registers". However if you look at
it:

static void integrator_clcd_enable(struct clcd_fb *fb)
{
        struct fb_var_screeninfo *var = &fb->fb.var;
        u32 val;

        dev_info(&fb->dev->dev, "enable Integrator CLCD connectors\n");

        val = INTEGRATOR_CLCD_LCD_STATIC1 | INTEGRATOR_CLCD_LCD_STATIC2 |
                INTEGRATOR_CLCD_LCD0_EN | INTEGRATOR_CLCD_LCD1_EN;
        if (var->bits_per_pixel <= 8 ||
            (var->bits_per_pixel == 16 && var->green.length == 5))
                /* Pseudocolor, RGB555, BGR555 */
                val |= INTEGRATOR_CLCD_LCDMUX_VGA555;
        else if (fb->fb.var.bits_per_pixel <= 16)
                /* truecolor RGB565 */
                val |= INTEGRATOR_CLCD_LCDMUX_VGA565;
        else
                val = 0; /* no idea for this, don't trust the docs */

        regmap_update_bits(versatile_syscon_map,
                           INTEGRATOR_HDR_CTRL_OFFSET,
                           0,
                           INTEGRATOR_CLCD_MASK);
}

This is stuff that is so closely tied in to the fbdev driver that even
if it is SoC-specific (and reside in arch/arm/mach-integrator etc
today) it would be hard to argument that it should not be part
of the fbdev driver: what it does is connect the lines out of the
CLCD block to the physical VGA encode chip in different ways
depending on how the pixels were set up.

Yours,
Linus Walleij
--
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
Linus Walleij Feb. 25, 2016, 3:36 p.m. | #13
On Thu, Feb 25, 2016 at 3:35 PM, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:
>> On Feb 25, 2016, at 15:43 , Linus Walleij <linus.walleij@linaro.org> wrote:


>> It also means there is no way to get the console up without the right

>> firmware files in the filesystem. I think that is really crap compared

>> to what we have today where the display will always come up, and

>> basically a regression.

>>

>> I understand the stance with respect to things like add-on hardware

>> like a Bluetooth board or WLAN or whatnot. But the fbdev console

>> is just too basic, like a serial port IMO.

>>

>> Sure in the ARM world we usually have a serial console, but this is

>> seriously breaking current practice.

>

> As Tomi mentioned firmware files can be located in the kernel image; there is no

> requirement to be in a filesystem, and that application can be performed really

> early, before even early init.


Are you thinking about exploiting an appended DT with
CONFIG_ARM_APPENDED_DTB or something else?

Yours,
Linus Walleij
--
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
Linus Walleij Feb. 25, 2016, 7:30 p.m. | #14
On Thu, Feb 25, 2016 at 5:45 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 25/02/16 16:04, Linus Walleij wrote:


>> *All* the others

>> will be very happy with *ONE* display defined as panel in the

>> device tree, and off they go. Usually VGA. And that will look

>

> You keep mentioning VGA. So is there are VGA output? Or do you just mean

> MIPI DPI panels, which happen to take the same video timings as VGA?


Russell beat me to it, yes there is an external VGA encoder.
It needs some bits set up through the "misc registers" system
controller as indicated. From the CLCD hardware point of view
it's no different than any other panel. So the DTS fragment looks
like so:

                        panel {
                                compatible = "panel-dpi";

                                port {
                                        clcd_panel: endpoint {
                                                remote-endpoint = <&clcd_pads>;
                                        };
                                };

                                /* Standard 640x480 VGA timings */
                                panel-timing {
                                        clock-frequency = <25175000>;
                                        hactive = <640>;
                                        hback-porch = <48>;
                                        hfront-porch = <16>;
                                        hsync-len = <96>;
                                        vactive = <480>;
                                        vback-porch = <33>;
                                        vfront-porch = <10>;
                                        vsync-len = <2>;
                                };
                        };


This is reported as the default display type if no LCD panel
is connected.

If a LCD panel is also connected, it take precedence.

Yours,
Linus Walleij
--
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
Linus Walleij Feb. 25, 2016, 7:32 p.m. | #15
On Thu, Feb 25, 2016 at 5:45 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> Anyway, if it's likely that we're not seeing new CLCD boards, I think

> it's fine if we don't go to any great lengths to clean things up there.

> Let's just get it working.


Do you think you could merge the other patch set I made, that doesn't
even deal with this plug-n-play-panel issue?

I have rebased the two approaches to autodetection on top of that.

Yours,
Linus Walleij
--
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
Linus Walleij March 5, 2016, 4:57 p.m. | #16
On Fri, Feb 26, 2016 at 5:47 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> Although one thing to consider is that if there is ever going to be a

> DRM driver for CLCD, it would be good to have the device tree parts

> correctly representing the hardware, so that the DRM driver could be

> implemented in a cleaner, more generic way.


OK so for the next kernel cycle I can work on that.

The non-controversial patch set is essentially just using the DT bindings
that are already in the kernel and in widespread use. See:
commit d10715be03bd8bad59ddc50236cb140c3bd73c7b
"video: ARM CLCD: Add DT support"

They follow the example set by
commit 478a4f81af4936c683a03488e15b087e28cb4f0d
"ARM: vexpress: Add CLCD Device Tree properties"

Yours,
Linus Walleij
--
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

Patch hide | download patch | download mbox

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index a4a6d70e8b26..76baaa51080c 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -743,6 +743,7 @@  dtb-$(CONFIG_ARCH_UNIPHIER) += \
 	uniphier-proxstream2-vodka.dtb
 dtb-$(CONFIG_ARCH_VERSATILE) += \
 	versatile-ab.dtb \
+	versatile-ab-ib2.dtb \
 	versatile-pb.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += \
 	vexpress-v2p-ca5s.dtb \
diff --git a/arch/arm/boot/dts/versatile-ab-ib2.dts b/arch/arm/boot/dts/versatile-ab-ib2.dts
new file mode 100644
index 000000000000..4b98b5382922
--- /dev/null
+++ b/arch/arm/boot/dts/versatile-ab-ib2.dts
@@ -0,0 +1,20 @@ 
+#include <versatile-ab.dts>
+
+/ {
+	model = "ARM Versatile AB + IB2 board";
+
+	/* Special IB2 control register */
+	ib2_syscon@27000000 {
+		compatible = "arm,versatile-ib2-syscon", "syscon", "simple-mfd";
+		reg = <0x27000000 0x4>;
+
+		led@00.4 {
+			compatible = "register-bit-led";
+			offset = <0x00>;
+			mask = <0x10>;
+			label = "versatile-ib2:0";
+			linux,default-trigger = "heartbeat";
+			default-state = "on";
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/versatile-ab.dts b/arch/arm/boot/dts/versatile-ab.dts
index 6fd7efbead34..836511cf8046 100644
--- a/arch/arm/boot/dts/versatile-ab.dts
+++ b/arch/arm/boot/dts/versatile-ab.dts
@@ -29,6 +29,147 @@ 
 		clock-frequency = <24000000>;
 	};
 
+	vga_panel {
+		compatible = "VGA", "panel-dpi";
+		port {
+			clcd_vga_panel: endpoint {
+				remote-endpoint = <&clcd_vga_pads>;
+			};
+		};
+
+		/* Standard 640x480 VGA timings */
+		panel-timing {
+			clock-frequency = <25175000>;
+			pixelclk-active = <0>;
+			hactive = <640>;
+			hback-porch = <48>;
+			hfront-porch = <16>;
+			hsync-len = <96>;
+			vactive = <480>;
+			vback-porch = <33>;
+			vfront-porch = <10>;
+			vsync-len = <2>;
+		};
+	};
+
+	xvga_panel {
+		compatible = "XVGA", "panel-dpi";
+		port {
+			clcd_xvga_panel: endpoint {
+				remote-endpoint = <&clcd_xvga_pads>;
+			};
+		};
+
+		/* Standard 1024x768 XVGA timings */
+		panel-timing {
+			clock-frequency = <63500127>;
+			pixelclk-active = <0>;
+			hactive = <1024>;
+			hback-porch = <152>;
+			hfront-porch = <48>;
+			hsync-len = <104>;
+			vactive = <768>;
+			vback-porch = <23>;
+			vfront-porch = <3>;
+			vsync-len = <4>;
+		};
+	};
+
+	sanyo38_panel {
+		compatible = "sanyo,tm38qv67a02a", "panel-dpi";
+		port {
+			clcd_sanyo38_panel: endpoint {
+				remote-endpoint = <&clcd_sanyo38_pads>;
+			};
+		};
+
+		panel-timing {
+			clock-frequency = <10000000>;
+			pixelclk-active = <1>;
+			hactive = <320>;
+			hback-porch = <6>;
+			hfront-porch = <6>;
+			hsync-len = <6>;
+			vactive = <240>;
+			vback-porch = <6>;
+			vfront-porch = <6>;
+			vsync-len = <0>;
+		};
+	};
+
+	sharp84_panel {
+		compatible = "sharp,lq084v1dg21", "panel-dpi";
+		port {
+			clcd_sharp84_panel: endpoint {
+				remote-endpoint = <&clcd_sharp84_pads>;
+			};
+		};
+
+		/* Standard 640x480 VGA timings */
+		panel-timing {
+			clock-frequency = <25175000>;
+			pixelclk-active = <0>;
+			hactive = <640>;
+			hback-porch = <48>;
+			hfront-porch = <16>;
+			hsync-len = <96>;
+			vactive = <480>;
+			vback-porch = <33>;
+			vfront-porch = <10>;
+			vsync-len = <2>;
+		};
+	};
+
+	/*
+	 * The IB2 has a Sanyo ALR252RGT QVGA panel mounted.
+	 */
+	sanyo25_panel {
+		compatible = "sanyo,alr252rgt", "panel-dpi";
+		port {
+			clcd_sanyo25_panel: endpoint {
+				remote-endpoint = <&clcd_sanyo25_pads>;
+			};
+		};
+
+		panel-timing {
+			clock-frequency = <5440000>;
+			pixelclk-active = <0>;
+			hsync-active = <0>;
+			vsync-active = <0>;
+			de-active = <1>;
+			hactive = <240>;
+			hback-porch = <20>;
+			hfront-porch = <10>;
+			hsync-len = <10>;
+			vactive = <320>;
+			vback-porch = <2>;
+			vfront-porch = <2>;
+			vsync-len = <2>;
+		};
+	};
+
+	epson_panel {
+		compatible = "epson,l2f50113t00", "panel-dpi";
+		port {
+			clcd_epson_panel: endpoint {
+				remote-endpoint = <&clcd_epson_pads>;
+			};
+		};
+
+		panel-timing {
+			clock-frequency = <16000000>;
+			pixelclk-active = <0>;
+			hactive = <176>;
+			hback-porch = <3>;
+			hfront-porch = <2>;
+			hsync-len = <3>;
+			vactive = <220>;
+			vback-porch = <1>;
+			vfront-porch = <0>;
+			vsync-len = <2>;
+		};
+	};
+
 	core-module@10000000 {
 		compatible = "arm,core-module-versatile", "syscon", "simple-mfd";
 		reg = <0x10000000 0x200>;
@@ -93,10 +234,20 @@ 
 			default-state = "off";
 		};
 
-		/* OSC1 on AB, OSC4 on PB */
-		osc1: cm_aux_osc@24M {
+		oscclk0: osc0@0c {
+			compatible = "arm,syscon-icst307";
 			#clock-cells = <0>;
-			compatible = "arm,versatile-cm-auxosc";
+			lock-offset = <0x20>;
+			vco-offset = <0x0C>;
+			clocks = <&xtal24mhz>;
+		};
+
+		/* This is called OSC1 on AB, OSC4 on PB, call it OSC4 here */
+		oscclk4: osc4@1c {
+			compatible = "arm,syscon-icst307";
+			#clock-cells = <0>;
+			lock-offset = <0x20>;
+			vco-offset = <0x1C>;
 			clocks = <&xtal24mhz>;
 		};
 
@@ -227,8 +378,52 @@ 
 			compatible = "arm,pl110", "arm,primecell";
 			reg = <0x10120000 0x1000>;
 			interrupts = <16>;
-			clocks = <&osc1>, <&pclk>;
+			clocks = <&oscclk4>, <&pclk>;
 			clock-names = "clcd", "apb_pclk";
+			/* 16bpp @ 25.175MHz = 50350000 bytes per second */
+			max-memory-bandwidth = <50350000>;
+
+			port {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				/*
+				 * The CLCD connects to either of these panels.
+				 * at run-time, board-specific code will
+				 * detect and select the right one. If no
+				 * panel is detected, the first one (VGA)
+				 * will be used by default.
+				 */
+				clcd_vga_pads: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&clcd_vga_panel>;
+					arm,pl11x,tft-r0g0b0-pads = <1 7 13>;
+				};
+				clcd_xvga_pads: endpoint@1 {
+					reg = <1>;
+					remote-endpoint = <&clcd_xvga_panel>;
+					arm,pl11x,tft-r0g0b0-pads = <1 7 13>;
+				};
+				clcd_sanyo38_pads: endpoint@2 {
+					reg = <2>;
+					remote-endpoint = <&clcd_sanyo38_panel>;
+					arm,pl11x,tft-r0g0b0-pads = <0 8 16>;
+				};
+				clcd_sharp84_pads: endpoint@3 {
+					reg = <3>;
+					remote-endpoint = <&clcd_sharp84_panel>;
+					arm,pl11x,tft-r0g0b0-pads = <0 8 16>;
+				};
+				clcd_sanyo25_pads: endpoint@4 {
+					reg = <4>;
+					remote-endpoint = <&clcd_sanyo25_panel>;
+					arm,pl11x,tft-r0g0b0-pads = <0 8 16>;
+				};
+				clcd_epson_pads: endpoint@5 {
+					reg = <5>;
+					remote-endpoint = <&clcd_epson_panel>;
+					arm,pl11x,tft-r0g0b0-pads = <0 8 16>;
+				};
+			};
 		};
 
 		sctl@101e0000 {
diff --git a/arch/arm/mach-versatile/versatile_dt.c b/arch/arm/mach-versatile/versatile_dt.c
index c44871851255..ebe27945fea5 100644
--- a/arch/arm/mach-versatile/versatile_dt.c
+++ b/arch/arm/mach-versatile/versatile_dt.c
@@ -29,8 +29,6 @@ 
 #include <linux/of_platform.h>
 #include <linux/slab.h>
 #include <linux/amba/bus.h>
-#include <linux/amba/clcd.h>
-#include <linux/platform_data/video-clcd-versatile.h>
 #include <linux/amba/mmci.h>
 #include <linux/mtd/physmap.h>
 #include <asm/mach-types.h>
@@ -57,7 +55,6 @@ 
 #define VERSATILE_SYS_PCICTL_OFFSET           0x44
 #define VERSATILE_SYS_MCI_OFFSET              0x48
 #define VERSATILE_SYS_FLASH_OFFSET            0x4C
-#define VERSATILE_SYS_CLCD_OFFSET             0x50
 
 /*
  * VERSATILE_SYS_FLASH
@@ -69,10 +66,7 @@ 
  */
 #define VERSATILE_MMCI0_BASE           0x10005000	/* MMC interface */
 #define VERSATILE_MMCI1_BASE           0x1000B000	/* MMC Interface */
-#define VERSATILE_CLCD_BASE            0x10120000	/* CLCD */
 #define VERSATILE_SCTL_BASE            0x101E0000	/* System controller */
-#define VERSATILE_IB2_BASE             0x24000000	/* IB2 module */
-#define VERSATILE_IB2_CTL_BASE		(VERSATILE_IB2_BASE + 0x03000000)
 
 /*
  * System controller bit assignment
@@ -86,7 +80,6 @@ 
 #define VERSATILE_TIMER4_EnSel	21
 
 static void __iomem *versatile_sys_base;
-static void __iomem *versatile_ib2_ctrl;
 
 static void versatile_flash_set_vpp(struct platform_device *pdev, int on)
 {
@@ -149,158 +142,6 @@  static struct mmci_platform_data mmc1_plat_data = {
 };
 
 /*
- * CLCD support.
- */
-#define SYS_CLCD_MODE_MASK	(3 << 0)
-#define SYS_CLCD_MODE_888	(0 << 0)
-#define SYS_CLCD_MODE_5551	(1 << 0)
-#define SYS_CLCD_MODE_565_RLSB	(2 << 0)
-#define SYS_CLCD_MODE_565_BLSB	(3 << 0)
-#define SYS_CLCD_NLCDIOON	(1 << 2)
-#define SYS_CLCD_VDDPOSSWITCH	(1 << 3)
-#define SYS_CLCD_PWR3V5SWITCH	(1 << 4)
-#define SYS_CLCD_ID_MASK	(0x1f << 8)
-#define SYS_CLCD_ID_SANYO_3_8	(0x00 << 8)
-#define SYS_CLCD_ID_UNKNOWN_8_4	(0x01 << 8)
-#define SYS_CLCD_ID_EPSON_2_2	(0x02 << 8)
-#define SYS_CLCD_ID_SANYO_2_5	(0x07 << 8)
-#define SYS_CLCD_ID_VGA		(0x1f << 8)
-
-static bool is_sanyo_2_5_lcd;
-
-/*
- * Disable all display connectors on the interface module.
- */
-static void versatile_clcd_disable(struct clcd_fb *fb)
-{
-	void __iomem *sys_clcd = versatile_sys_base + VERSATILE_SYS_CLCD_OFFSET;
-	u32 val;
-
-	val = readl(sys_clcd);
-	val &= ~SYS_CLCD_NLCDIOON | SYS_CLCD_PWR3V5SWITCH;
-	writel(val, sys_clcd);
-
-	/*
-	 * If the LCD is Sanyo 2x5 in on the IB2 board, turn the back-light off
-	 */
-	if (of_machine_is_compatible("arm,versatile-ab") && is_sanyo_2_5_lcd) {
-		unsigned long ctrl;
-
-		ctrl = readl(versatile_ib2_ctrl);
-		ctrl &= ~0x01;
-		writel(ctrl, versatile_ib2_ctrl);
-	}
-}
-
-/*
- * Enable the relevant connector on the interface module.
- */
-static void versatile_clcd_enable(struct clcd_fb *fb)
-{
-	struct fb_var_screeninfo *var = &fb->fb.var;
-	void __iomem *sys_clcd = versatile_sys_base + VERSATILE_SYS_CLCD_OFFSET;
-	u32 val;
-
-	val = readl(sys_clcd);
-	val &= ~SYS_CLCD_MODE_MASK;
-
-	switch (var->green.length) {
-	case 5:
-		val |= SYS_CLCD_MODE_5551;
-		break;
-	case 6:
-		if (var->red.offset == 0)
-			val |= SYS_CLCD_MODE_565_RLSB;
-		else
-			val |= SYS_CLCD_MODE_565_BLSB;
-		break;
-	case 8:
-		val |= SYS_CLCD_MODE_888;
-		break;
-	}
-
-	/*
-	 * Set the MUX
-	 */
-	writel(val, sys_clcd);
-
-	/*
-	 * And now enable the PSUs
-	 */
-	val |= SYS_CLCD_NLCDIOON | SYS_CLCD_PWR3V5SWITCH;
-	writel(val, sys_clcd);
-
-	/*
-	 * If the LCD is Sanyo 2x5 in on the IB2 board, turn the back-light on
-	 */
-	if (of_machine_is_compatible("arm,versatile-ab") && is_sanyo_2_5_lcd) {
-		unsigned long ctrl;
-
-		ctrl = readl(versatile_ib2_ctrl);
-		ctrl |= 0x01;
-		writel(ctrl, versatile_ib2_ctrl);
-	}
-}
-
-/*
- * Detect which LCD panel is connected, and return the appropriate
- * clcd_panel structure.  Note: we do not have any information on
- * the required timings for the 8.4in panel, so we presently assume
- * VGA timings.
- */
-static int versatile_clcd_setup(struct clcd_fb *fb)
-{
-	void __iomem *sys_clcd = versatile_sys_base + VERSATILE_SYS_CLCD_OFFSET;
-	const char *panel_name;
-	u32 val;
-
-	is_sanyo_2_5_lcd = false;
-
-	val = readl(sys_clcd) & SYS_CLCD_ID_MASK;
-	if (val == SYS_CLCD_ID_SANYO_3_8)
-		panel_name = "Sanyo TM38QV67A02A";
-	else if (val == SYS_CLCD_ID_SANYO_2_5) {
-		panel_name = "Sanyo QVGA Portrait";
-		is_sanyo_2_5_lcd = true;
-	} else if (val == SYS_CLCD_ID_EPSON_2_2)
-		panel_name = "Epson L2F50113T00";
-	else if (val == SYS_CLCD_ID_VGA)
-		panel_name = "VGA";
-	else {
-		printk(KERN_ERR "CLCD: unknown LCD panel ID 0x%08x, using VGA\n",
-			val);
-		panel_name = "VGA";
-	}
-
-	fb->panel = versatile_clcd_get_panel(panel_name);
-	if (!fb->panel)
-		return -EINVAL;
-
-	return versatile_clcd_setup_dma(fb, SZ_1M);
-}
-
-static void versatile_clcd_decode(struct clcd_fb *fb, struct clcd_regs *regs)
-{
-	clcdfb_decode(fb, regs);
-
-	/* Always clear BGR for RGB565: we do the routing externally */
-	if (fb->fb.var.green.length == 6)
-		regs->cntl &= ~CNTL_BGR;
-}
-
-static struct clcd_board clcd_plat_data = {
-	.name		= "Versatile",
-	.caps		= CLCD_CAP_5551 | CLCD_CAP_565 | CLCD_CAP_888,
-	.check		= clcdfb_check,
-	.decode		= versatile_clcd_decode,
-	.disable	= versatile_clcd_disable,
-	.enable		= versatile_clcd_enable,
-	.setup		= versatile_clcd_setup,
-	.mmap		= versatile_clcd_mmap_dma,
-	.remove		= versatile_clcd_remove_dma,
-};
-
-/*
  * Lookup table for attaching a specific name and platform_data pointer to
  * devices as they get created by of_platform_populate().  Ideally this table
  * would not exist, but the current clock implementation depends on some devices
@@ -309,7 +150,6 @@  static struct clcd_board clcd_plat_data = {
 struct of_dev_auxdata versatile_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("arm,primecell", VERSATILE_MMCI0_BASE, "fpga:05", &mmc0_plat_data),
 	OF_DEV_AUXDATA("arm,primecell", VERSATILE_MMCI1_BASE, "fpga:0b", &mmc1_plat_data),
-	OF_DEV_AUXDATA("arm,primecell", VERSATILE_CLCD_BASE, "dev:20", &clcd_plat_data),
 	{}
 };
 
@@ -400,8 +240,6 @@  static void __init versatile_dt_init(void)
 		versatile_sys_base = of_iomap(np, 0);
 	WARN_ON(!versatile_sys_base);
 
-	versatile_ib2_ctrl = ioremap(VERSATILE_IB2_CTL_BASE, SZ_4K);
-
 	versatile_dt_pci_init();
 
 	platform_device_register(&versatile_flash_device);