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