On 02/11/2020 16:16, Rob Herring wrote:
> On Fri, Oct 30, 2020 at 4:15 PM Sam Ravnborg <[email protected]> wrote:
>>
>> Hi Neil.
>>
>> On Fri, Oct 30, 2020 at 09:31:36AM +0100, Neil Armstrong wrote:
>>> 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 <[email protected]>
>>>>> Cc: Sam Ravnborg <[email protected]>
>>>>> Cc: Thomas Zimmermann <[email protected]>
>>>>> Cc: Daniel Vetter <[email protected]>
>>>>
>>>> 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 <[email protected]>
>>>>
>>>>> ---
>>>>> .../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 <[email protected]>
>>>>> + - Edmond J Dea <[email protected]>
>>>>> +
>>>>> +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/by5pr11mb41827de07436dd0454e24e6e8c...@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.
>>
>> I think I got it now.
>>
>> msscam is used to enable clocks both for the display driver and the
>> mipi-dsi part.
>
> If just clocks, then the msscam should be a clock provider possibly.
> If not, then the below looks right.
I agree
>
>>
>> So it should be pulled in to a dedicated node - for example like this:
>>
>> mssscam: msscam@20910000 {
>> compatible = "intel,keembay-msscam", "syscon";
>> reg = <0x20910000, 0x30>;
>> reg-io-width = <4>;
>> };
>>
>> And ofc we need a binding file for that.
>>
>>
>> And then we could have code like this in the display driver:
>> regmap *msscam =
>> syscon_regmap_lookup_by_compatible("intel,keembay-msscam");
>> if (IS_ERR(msscam))
>> tell-and goto-out;
It's better to use a phandle in the display node, instead of looking for
compatible nodes.
>>
>> regmap_write(msscam, MSS_LCD_MIPI_CFG, 1);
>> regmap_write(msscam, MSS_LOOPBACK_CFG, 0);
>>
>> And then in the kmb_dsi part:
>> regmap *msscam =
>> syscon_regmap_lookup_by_compatible("intel,keembay-msscam");
>> if (IS_ERR(msscam))
>> tell-and goto-out;
>>
>> regmap_write(msscam, MSS_MIPI_CIF_CFG, 1);
>>
>>
>> Did I finally get it?
Yes, this is what I was thinking about, thanks
Neil
>>
>> Sam
>> _______________________________________________
>> dri-devel mailing list
>> [email protected]
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
