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