[PATCH v10 1/6] dt-bindings: display: Add support for Intel KeemBay Display

[Neil Armstrong]

Hi,

On 29/10/2020 23:20, Sam Ravnborg wrote:
> Hi Anitha.
> 
> On Thu, Oct 29, 2020 at 02:27:52PM -0700, Anitha Chrisanthus wrote:
>> This patch adds bindings for Intel KeemBay Display
>>
>> v2: review changes from Rob Herring
>> v3: review changes from Sam Ravnborg (removed mipi dsi entries, and
>>     encoder entry, connect port to dsi)
>>     MSSCAM is part of the display submodule and its used to reset LCD
>>     and MIPI DSI clocks, so its best to be on this device tree.
>>
>> Signed-off-by: Anitha Chrisanthus <anitha.chrisanthus at intel.com>
>> Cc: Sam Ravnborg <sam at ravnborg.org>
>> Cc: Thomas Zimmermann <tzimmermann at suse.de>
>> Cc: Daniel Vetter <daniel at ffwll.ch>
> 
> Looks good - and the split betwwen the display and the mipi<->dsi parts
> matches the understanding of the HW I have developed.
> 
> Reviewed-by: Sam Ravnborg <sam at ravnborg.org>
> 
>> ---
>>  .../bindings/display/intel,keembay-display.yaml    | 75 ++++++++++++++++++++++
>>  1 file changed, 75 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/display/intel,keembay-display.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/display/intel,keembay-display.yaml b/Documentation/devicetree/bindings/display/intel,keembay-display.yaml
>> new file mode 100644
>> index 0000000..8a8effe
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/intel,keembay-display.yaml
>> @@ -0,0 +1,75 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/intel,keembay-display.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Devicetree bindings for Intel Keem Bay display controller
>> +
>> +maintainers:
>> +  - Anitha Chrisanthus <anitha.chrisanthus at intel.com>
>> +  - Edmond J Dea <edmund.j.dea at intel.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: intel,keembay-display
>> +
>> +  reg:
>> +    items:
>> +      - description: LCD registers range
>> +      - description: Msscam registers range
>> +

Indeed the split is much better, but as you replied on http://lore.kernel.org/r/BY5PR11MB41827DE07436DD0454E24E6E8C0A0@BY5PR11MB4182.namprd11.prod.outlook.com
the msscam seems to be shared with the camera subsystem block, if this is the case it should be handled.

If it's a shared register block, it could be defined as a "syscon" used by both subsystems.

Neil


>> +  reg-names:
>> +    items:
>> +      - const: lcd
>> +      - const: msscam
>> +
>> +  clocks:
>> +    items:
>> +      - description: LCD controller clock
>> +      - description: pll0 clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: clk_lcd
>> +      - const: clk_pll0
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  port:
>> +    type: object
>> +    description: Display output node to DSI.
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - reg-names
>> +  - clocks
>> +  - clock-names
>> +  - interrupts
>> +  - port
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +    display at 20930000 {
>> +        compatible = "intel,keembay-display";
>> +        reg = <0x20930000 0x3000>,
>> +              <0x20910000 0x30>;
>> +        reg-names = "lcd", "msscam";
>> +        interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
>> +        clocks = <&scmi_clk 0x83>,
>> +                 <&scmi_clk 0x0>;
>> +        clock-names = "clk_lcd", "clk_pll0";
>> +
>> +        port {
>> +            disp_out: endpoint {
>> +                remote-endpoint = <&dsi_in>;
>> +            };
>> +        };
>> +    };
>> -- 
>> 2.7.4
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>