On 02/11/2020 16:16, Rob Herring wrote: > On Fri, Oct 30, 2020 at 4:15 PM Sam Ravnborg <s...@ravnborg.org> 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.chrisant...@intel.com> >>>>> Cc: Sam Ravnborg <s...@ravnborg.org> >>>>> Cc: Thomas Zimmermann <tzimmerm...@suse.de> >>>>> Cc: Daniel Vetter <dan...@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 <s...@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.chrisant...@intel.com> >>>>> + - Edmond J Dea <edmund.j....@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/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 >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel