Re: [RFC PATCH 0/7] add at91sam9 LCDC DRM driver

[Sam Ravnborg <sam@ravnborg.org>]

Hi Rob.

> I don't know that 2 registers for a backlight PWM constitute an MFD. A 
> single node can be both an LCD controller and a PWM.

Current suggestion from v1 patchset looks like this:
	lcdc0: lcdc@700000 {
		compatible = "atmel,at91sam9263-lcdc-mfd";
		reg = <0x700000 0x1000>;
		interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>;
		clocks = <&lcd_clk>, <&lcd_clk>;
		clock-names = "lcdc_clk", "hclk";

		lcdc-display-controller {
			compatible = "atmel,lcdc-display-controller";
			lcd-supply = <&lcdc_reg>;

			port@0 {
			};
		};

		lcdc_pwm: lcdc-pwm {
			compatible = "atmel,lcdc-pwm";
			#pwm-cells = <3>;
		};

	};

        backlight: backlight {
                compatible = "pwm-backlight";
                pwms = <&lcdc_pwm 0 50000 0>;
        };


We could have described the full binding in one file, rather than in
three files like it is done in v1. But the structure was done so it matched
what was done for hlcdc.


If I understand the proposal from you correct an example binding would
look like this:

       lcdc0: lcdc@700000 {
                compatible = "atmel,at91sam9263-lcdc";
                reg = <0x700000 0x1000>;
                interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>;
                clocks = <&lcd_clk>, <&lcd_clk>;
                clock-names = "lcdc_clk", "hclk";

                lcd-supply = <&lcdc_reg>;

                port@0 {
                };

                #pwm-cells = <3>;
        };

        backlight: backlight {
                compatible = "pwm-backlight";
                pwms = <&lcd0 0 50000 0>;
        };


This is doable, but IMO it is less obvious that the LCDC IP core implements
two different features - a PWM and a LCD controller.
Right now the preference is to stay with the v1 approach:
- It is a mirror of what we do today for hlcdc, so no suprises
- It shows in a nice way that the LCDC IP core implments both an LCD controller and a PWM
- The pwm functionality is not hiddin inside the lcdc stuff, and it is thus
  simpler to add good pin-ctrl handles with nice names that matches the usage.
  (I could always add more pin-ctrl, but it is common to refer to a single pin-ctrl.


One DT related Q:
The LCD Controller supports BGR565, but as this is less common some HW implmentations
exchange R and B, expessed in the old binding as wiring-mode like this:

	atmel,lcd-wiring-mode: lcd wiring mode "RGB" or "BRG"

How can we express this wiring-mode in a generic way, both in DT and in code?
Is it something that in DRM belongs to the panel, the encoder, the connector, or?
And can any of the exisitng flags be used?

Thanks in advance,

	Sam