Re: [PATCH v11 3/5] soc: mediatek: Move mt8173 MMSYS to platform driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Enric Balletbo i Serra (2020-03-06 08:30:16)
> Hi Stephen,
> 
> On 5/3/20 22:01, Stephen Boyd wrote:
> > Quoting Enric Balletbo i Serra (2020-03-02 03:01:26)
> >> From: Matthias Brugger <mbrugger@xxxxxxxx>
> >>
> >> There is no strong reason for this to use CLK_OF_DECLARE instead of
> >> being a platform driver.
> > 
> > Cool.
> > 
> >> Plus, this driver provides clocks but also
> >> a shared register space for the mediatek-drm and the mediatek-mdp
> >> driver. So move to drivers/soc/mediatek as a platform driver.
> >>
> >> Signed-off-by: Matthias Brugger <mbrugger@xxxxxxxx>
> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@xxxxxxxxxxxxx>
> >> Reviewed-by: CK Hu <ck.hu@xxxxxxxxxxxx>
> >> ---
> >>
> >> Changes in v11: None
> >> Changes in v10:
> >> - Renamed to be generic mtk-mmsys
> >> - Add driver data support to be able to support diferent SoCs
> >>
> >> Changes in v9:
> >> - Move mmsys to drivers/soc/mediatek (CK)
> >>
> >> Changes in v8:
> >> - Be a builtin_platform_driver like other mediatek mmsys drivers.
> >>
> >> Changes in v7:
> >> - Free clk_data->clks as well
> >> - Get rid of private data structure
> >>
> >>  drivers/clk/mediatek/clk-mt8173.c | 104 --------------------
> >>  drivers/soc/mediatek/Kconfig      |   7 ++
> >>  drivers/soc/mediatek/Makefile     |   1 +
> >>  drivers/soc/mediatek/mtk-mmsys.c  | 154 ++++++++++++++++++++++++++++++
> > 
> > Can you generate with -M so that we can see what has actually changed?
> > 
> 
> Sure, sorry about that.
> 
> >>  4 files changed, 162 insertions(+), 104 deletions(-)
> >>  create mode 100644 drivers/soc/mediatek/mtk-mmsys.c
> >>
> >> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> >> index 2114b563478c..7a156944d50e 100644
> >> --- a/drivers/soc/mediatek/Kconfig
> >> +++ b/drivers/soc/mediatek/Kconfig
> >> @@ -44,4 +44,11 @@ config MTK_SCPSYS
> >>           Say yes here to add support for the MediaTek SCPSYS power domain
> >>           driver.
> >>  
> >> +config MTK_MMSYS
> >> +       bool "MediaTek MMSYS Support"
> >> +       depends on COMMON_CLK_MT8173
> > 
> > Does it need some default so that defconfig updates don't break things?
> > 
> 
> Right.
> 
> >> +       help
> >> +         Say yes here to add support for the MediaTek Multimedia
> >> +         Subsystem (MMSYS).
> >> +
> >>  endmenu
> >> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> >> index b01733074ad6..01f9f873634a 100644
> >> --- a/drivers/soc/mediatek/Makefile
> >> +++ b/drivers/soc/mediatek/Makefile
> >> @@ -3,3 +3,4 @@ obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o
> >>  obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
> >>  obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
> >>  obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
> >> +obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o
> >> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> >> new file mode 100644
> >> index 000000000000..473cdf732fb5
> >> --- /dev/null
> >> +++ b/drivers/soc/mediatek/mtk-mmsys.c
> >> @@ -0,0 +1,154 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/*
> >> + * Copyright (c) 2014 MediaTek Inc.
> >> + * Author: James Liao <jamesjj.liao@xxxxxxxxxxxx>
> >> + */
> >> +
> >> +#include <linux/clk-provider.h>
> >> +#include <linux/of_device.h>
> >> +#include <linux/platform_device.h>
> >> +
> >> +#include "../../clk/mediatek/clk-gate.h"
> >> +#include "../../clk/mediatek/clk-mtk.h"
> > 
> > Why not use include/linux/clk/?
> > 
> 
> I can move these files to include, this will impact a lot more of drivers but,
> yes, I think is the right way.
> 
> > But I also don't understand why the clk driver is moved outside of
> > drivers/clk/ into drivers/soc/. Commit text saying that it has shared
> > registers doesn't mean it can't still keep the clk driver part in the
> > drivers/clk/ area.
> > 
> 
> Actually moving this to the soc directory has been requested by CK (mediatek) as
> a change in v8. You can see the discussion in [1]
> 

I can reply there in that thread if necessary, but we shouldn't need to
force simple-mfd into DT bindings to support this. Match the compatible
string in drivers/soc/ and register devices in software for the
different pieces of this overall hardware block. If necessary, pass down
the ioremapped addresss down through device data to each logical driver
in the respective subsystem.

So yes, it looks like an MFD, but that doesn't mean we have to change
the DT binding or put it in drivers/mfd to support that. And we don't
have to fix any problems with allowing two drivers to probe the same
compatible string.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux