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. 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; 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? Sam _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
