| Message ID | 20200213201953.15268-1-matthias.bgg@kernel.org |
|---|---|
| Headers | show |
| Series | arm/arm64: mediatek: Fix mmsys device probing | expand |
Hi, Matthias: On Thu, 2020-02-13 at 21:19 +0100, matthias.bgg@kernel.org wrote: > From: Matthias Brugger <mbrugger@suse.com> > > Switch probing for the MMSYS to support invocation to a > plain paltform device. The driver will be probed by the DRM subsystem. > > Signed-off-by: Matthias Brugger <mbrugger@suse.com> > > --- > > Changes in v7: > - free clk_data->clks as well > - get rid of private data structure > > Changes in v6: None > Changes in v5: None > Changes in v4: None > Changes in v3: None > Changes in v2: None > > drivers/clk/mediatek/clk-mt8183-mm.c | 30 ++++++++++++++++++---------- > 1 file changed, 20 insertions(+), 10 deletions(-) > > diff --git a/drivers/clk/mediatek/clk-mt8183-mm.c b/drivers/clk/mediatek/clk-mt8183-mm.c > index 720c696b506d..7576cd231be3 100644 > --- a/drivers/clk/mediatek/clk-mt8183-mm.c > +++ b/drivers/clk/mediatek/clk-mt8183-mm.c > @@ -3,8 +3,10 @@ > // Copyright (c) 2018 MediaTek Inc. > // Author: Weiyi Lu <weiyi.lu@mediatek.com> > > +#include <linux/module.h> > #include <linux/clk-provider.h> > #include <linux/platform_device.h> > +#include <linux/slab.h> > > #include "clk-mtk.h" > #include "clk-gate.h" > @@ -85,27 +87,35 @@ static const struct mtk_gate mm_clks[] = { > static int clk_mt8183_mm_probe(struct platform_device *pdev) > { > struct clk_onecell_data *clk_data; > - struct device_node *node = pdev->dev.of_node; > + struct device_node *node = pdev->dev.parent->of_node; > + > + clk_data = devm_kzalloc(&pdev->dev, sizeof(*clk_data), GFP_KERNEL); I think this is redundant. > + if (!clk_data) > + return -ENOMEM; > > clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK); > + platform_set_drvdata(pdev, clk_data); > > - mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), > - clk_data); > + mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks), clk_data); > > return of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); > } > > -static const struct of_device_id of_match_clk_mt8183_mm[] = { > - { .compatible = "mediatek,mt8183-mmsys", }, > - {} > -}; > +static int clk_mt8183_mm_remove(struct platform_device *pdev) > +{ > + struct clk_onecell_data *clk_data = platform_get_drvdata(pdev); > + > + kfree(clk_data->clks); > + kfree(clk_data); These two statement looks like a reverse of mtk_alloc_clk_data() and exist in many files. It is worth to have a function (maybe mtk_free_clk_data()) to do this. In addition, should we undo what is done in clk_mt8183_mm_probe() such as mtk_clk_register_gates() and of_clk_add_provider()? Regards, CK > + > + return 0; > +} > > static struct platform_driver clk_mt8183_mm_drv = { > .probe = clk_mt8183_mm_probe, > + .remove = clk_mt8183_mm_remove, > .driver = { > .name = "clk-mt8183-mm", > - .of_match_table = of_match_clk_mt8183_mm, > }, > }; > - > -builtin_platform_driver(clk_mt8183_mm_drv); > +module_platform_driver(clk_mt8183_mm_drv);
On Thu, Feb 13, 2020 at 09:19:42PM +0100, matthias.bgg@kernel.org wrote: > From: Matthias Brugger <mbrugger@suse.com> > > The MediaTek DRM has a block called mmsys, which sets > the routing and enables the different blocks. > This patch adds one line for the mmsys bindings description and changes > the mmsys description to use the generic form of referring to a specific > Soc. > > Signed-off-by: Matthias Brugger <mbrugger@suse.com> > > --- > > Changes in v7: > - add hint to the mmsys binding document > - make mmsys description generic > - fix typo in commit message > > Changes in v6: None > Changes in v5: None > Changes in v4: None > Changes in v3: None > Changes in v2: None > > .../bindings/display/mediatek/mediatek,disp.txt | 3 +++ > .../bindings/display/mediatek/mediatek,mmsys.txt | 9 +-------- > 2 files changed, 4 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > index b91e709db7a4..8e453026ef78 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > @@ -24,6 +24,7 @@ connected to. > For a description of the display interface sink function blocks, see > Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.txt and > Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt. > +Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt. > > Required properties (all function blocks): > - compatible: "mediatek,<chip>-disp-<function>", one of > @@ -43,7 +44,9 @@ Required properties (all function blocks): > "mediatek,<chip>-dpi" - DPI controller, see mediatek,dpi.txt > "mediatek,<chip>-disp-mutex" - display mutex > "mediatek,<chip>-disp-od" - overdrive > + "mediatek,<chip>-mmsys", "syscon" - provide clocks and components management > the supported chips are mt2701, mt2712 and mt8173. > + > - reg: Physical base address and length of the function block register space > - interrupts: The interrupt signal from the function block (required, except for > merge and split function blocks). > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt > index 301eefbe1618..7bbadee820e3 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt > @@ -5,14 +5,7 @@ The Mediatek mmsys controller provides various clocks to the system. > > Required Properties: > > -- compatible: Should be one of: > - - "mediatek,mt2701-mmsys", "syscon" > - - "mediatek,mt2712-mmsys", "syscon" > - - "mediatek,mt6779-mmsys", "syscon" > - - "mediatek,mt6797-mmsys", "syscon" > - - "mediatek,mt7623-mmsys", "mediatek,mt2701-mmsys", "syscon" You've lost this information about the fallback... > - - "mediatek,mt8173-mmsys", "syscon" > - - "mediatek,mt8183-mmsys", "syscon" > +- compatible: "mediatek,<chip>-mmsys" You are just going to have to add these all back when this is converted to schema. Rob
Hi Matthias, On 2/13/20 9:19 PM, matthias.bgg@kernel.org wrote: > From: Matthias Brugger <mbrugger@suse.com> > > Check the return value of of_clk_get and print an error > message if not EPROBE_DEFER. > > Signed-off-by: Matthias Brugger <mbrugger@suse.com> This patch is independent from the remainder of this series, right? It looks good to me, so is it OK if I merge this in the media subsystem? Regards, Hans > > --- > > Changes in v7: > - fix check of return value of of_clk_get > - fix identation > > Changes in v6: None > Changes in v5: None > Changes in v4: None > Changes in v3: None > Changes in v2: None > > drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c > index 0c4788af78dd..58abfbdfb82d 100644 > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c > @@ -110,6 +110,12 @@ int mtk_mdp_comp_init(struct device *dev, struct device_node *node, > > for (i = 0; i < ARRAY_SIZE(comp->clk); i++) { > comp->clk[i] = of_clk_get(node, i); > + if (IS_ERR(comp->clk[i])) { > + if (PTR_ERR(comp->clk[i]) != -EPROBE_DEFER) > + dev_err(dev, "Failed to get clock\n"); > + > + return PTR_ERR(comp->clk[i]); > + } > > /* Only RDMA needs two clocks */ > if (comp->type != MTK_MDP_RDMA) >
On 24/02/2020 18:36, Hans Verkuil wrote: > Hi Matthias, > > On 2/13/20 9:19 PM, matthias.bgg@kernel.org wrote: >> From: Matthias Brugger <mbrugger@suse.com> >> >> Check the return value of of_clk_get and print an error >> message if not EPROBE_DEFER. >> >> Signed-off-by: Matthias Brugger <mbrugger@suse.com> > > This patch is independent from the remainder of this series, right? > It looks good to me, so is it OK if I merge this in the media subsystem? > Yes it is independent. Please merge it to the media subsystem. Thanks, Matthias > Regards, > > Hans > >> >> --- >> >> Changes in v7: >> - fix check of return value of of_clk_get >> - fix identation >> >> Changes in v6: None >> Changes in v5: None >> Changes in v4: None >> Changes in v3: None >> Changes in v2: None >> >> drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c >> index 0c4788af78dd..58abfbdfb82d 100644 >> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c >> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c >> @@ -110,6 +110,12 @@ int mtk_mdp_comp_init(struct device *dev, struct device_node *node, >> >> for (i = 0; i < ARRAY_SIZE(comp->clk); i++) { >> comp->clk[i] = of_clk_get(node, i); >> + if (IS_ERR(comp->clk[i])) { >> + if (PTR_ERR(comp->clk[i]) != -EPROBE_DEFER) >> + dev_err(dev, "Failed to get clock\n"); >> + >> + return PTR_ERR(comp->clk[i]); >> + } >> >> /* Only RDMA needs two clocks */ >> if (comp->type != MTK_MDP_RDMA) >> >
From: Matthias Brugger <mbrugger@suse.com> This is version seven of the series. The biggest change is, that I added a first patch that actually moves the mmsys binding from arm/mediatek to display/mediatek, as in effect the mmsys is part of the display subsystem. Since version five, the clock probing is implemented through a platform driver. The corresponding platform device get's created in the DRM driver. I converted all the clock drivers to platform drivers and tested the approach on the Acer Chromebook R13 (mt8173 based). Apart from that I reordered the patches so that the DT bindings update are the first patches. MMSYS in Mediatek SoCs has some registers to control clock gates (which is used in the clk driver) and some registers to set the routing and enable the differnet blocks of the display subsystem. Up to now both drivers, clock and drm are probed with the same device tree compatible. But only the first driver get probed, which in effect breaks graphics on mt8173 and mt2701. This patch uses a platform device registration in the DRM driver, which will trigger the probe of the corresponding clock driver. It was tested on the Acer R13 Chromebook. Changes in v7: - move the binding description - add hint to the mmsys binding document - make mmsys description generic - fix typo in commit message - fix check of return value of of_clk_get - free clk_data->clks as well - get rid of private data structure Changes in v6: - re-arrange the patch order - generate platform_device for mmsys clock driver inside the DRM driver - fix DTS binding accordingly - switch all mmsys clock driver to platform probing - fix mt8173 platform driver remove function - fix probe defer path in HDMI driver - fix probe defer path in mtk_mdp_comp - fix identation of error messages Changes in v5: - fix missing regmap accessors in drm diver (patch 1) - omit probe deffered warning on all drivers (patch 5) - update drm and clk bindings (patch 6 and 7) - put mmsys clock part in dts child node of mmsys. Only done for HW where no dts backport compatible breakage is expected (either DRM driver not yet implemented or no HW available to the public) (patch 9 to 12) Changes in v4: - use platform device to probe clock driver - add Acked-by CK Hu for the probe deferred patch Changes in v3: - fix kconfig typo (shame on me) - delete __initconst from mm_clocks as converted to a platform driver Changes in v2: - add binding documentation - ddp: use regmap_update_bits - ddp: ignore EPROBE_DEFER on clock probing - mfd: delete mmsys_private - add Reviewed-by and Acked-by tags Matthias Brugger (13): dt-bindings: arm: move mmsys description to display dt-bindings: display: mediatek: Add mmsys binding description dt-bindings: mediatek: Add compatible for mt7623 drm/mediatek: Use regmap for register access drm: mediatek: Omit warning on probe defers media: mtk-mdp: Check return value of of_clk_get clk: mediatek: mt2701: switch mmsys to platform device probing clk: mediatek: mt2712e: switch to platform device probing clk: mediatek: mt6779: switch mmsys to platform device probing clk: mediatek: mt6797: switch to platform device probing clk: mediatek: mt8183: switch mmsys to platform device probing clk: mediatek: mt8173: switch mmsys to platform device probing drm/mediatek: Add support for mmsys through a pdev .../display/mediatek/mediatek,disp.txt | 5 ++ .../mediatek/mediatek,mmsys.txt | 9 +--- drivers/clk/mediatek/clk-mt2701-mm.c | 34 ++++++++---- drivers/clk/mediatek/clk-mt2712-mm.c | 32 +++++++---- drivers/clk/mediatek/clk-mt6779-mm.c | 32 +++++++---- drivers/clk/mediatek/clk-mt6797-mm.c | 34 ++++++++---- drivers/clk/mediatek/clk-mt8173.c | 45 +++++++++++++--- drivers/clk/mediatek/clk-mt8183-mm.c | 30 +++++++---- drivers/gpu/drm/mediatek/mtk_disp_color.c | 5 +- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 5 +- drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 5 +- drivers/gpu/drm/mediatek/mtk_dpi.c | 12 +++-- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 4 +- drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 53 ++++++++----------- drivers/gpu/drm/mediatek/mtk_drm_ddp.h | 4 +- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 35 +++++++++--- drivers/gpu/drm/mediatek/mtk_drm_drv.h | 4 +- drivers/gpu/drm/mediatek/mtk_dsi.c | 8 ++- drivers/gpu/drm/mediatek/mtk_hdmi.c | 4 +- drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 6 +++ 20 files changed, 246 insertions(+), 120 deletions(-) rename Documentation/devicetree/bindings/{arm => display}/mediatek/mediatek,mmsys.txt (61%)