On 03/11/2020 03:02, Rob Herring wrote: > On Mon, Nov 2, 2020 at 10:38 AM Neil Armstrong <narmstrong@xxxxxxxxxxxx> wrote: >> >> On 02/11/2020 16:16, Rob Herring wrote: >>> On Fri, Oct 30, 2020 at 4:15 PM Sam Ravnborg <sam@xxxxxxxxxxxx> 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 <anitha.chrisanthus@xxxxxxxxx> >>>>>>> Cc: Sam Ravnborg <sam@xxxxxxxxxxxx> >>>>>>> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx> >>>>>>> Cc: Daniel Vetter <daniel@xxxxxxxx> >>>>>> >>>>>> 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@xxxxxxxxxxxx> >>>>>> >>>>>>> --- >>>>>>> .../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@xxxxxxxxx> >>>>>>> + - Edmond J Dea <edmund.j.dea@xxxxxxxxx> >>>>>>> + >>>>>>> +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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx >>>>> 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. > > No, you don't need it in DT unless there's more than one instance or > other parameters needed like register offsets/masks. The above is > actually faster too walking a list rather than a phandle look up > (though the phandle cache now may make it faster). A phandle makes the dependency explicit, anyway it's only an advice. Neil > > Rob > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
