Re: [PATCH v1 1/2] drm/mediatek: separate mipi_tx to different file

2019-02-19 Thread CK Hu
Hi, Jitao:

On Tue, 2019-02-19 at 17:14 +0800, Jitao Shi wrote:
> Different IC has different mipi_tx setting of dsi.
> This patch separates the mipi_tx hardware relate part for mt8173.
> 
> Signed-off-by: Jitao Shi 
> ---
>  drivers/gpu/drm/mediatek/Makefile |   1 +
>  drivers/gpu/drm/mediatek/mtk_mipi_tx.c| 350 ++
>  drivers/gpu/drm/mediatek/mtk_mipi_tx.h|  51 +++
>  drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c | 290 +++
>  4 files changed, 377 insertions(+), 315 deletions(-)
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_mipi_tx.h
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c
> 

[snip]
>  
> +static void mtk_mipi_tx_clk_get_ops(struct mtk_mipi_tx *mipi_tx,
> + const struct clk_ops **ops)
> +{
> + if (mipi_tx && mipi_tx->driver_data &&
> + mipi_tx->driver_data->mipi_tx_clk_ops)
> + *ops = mipi_tx->driver_data->mipi_tx_clk_ops;
> + else
> + dev_err(mipi_tx->dev, "Failed to get clk ops of phy\n");

Any where call mtk_mipi_tx_clk_get_ops() would make sure mipi_tx is not
NULL, so you need not to check mipi_tx. And the checking of driver_data
and mipi_tx_clk_ops looks unnecessary because any one who implement this
driver would never let this happen. So this function just need do the
assignment and therefore this assignment could just do in probe().

Regards,
CK

> +}
> +
>  static int mtk_mipi_tx_probe(struct platform_device *pdev)
>  {
>   struct device *dev = &pdev->dev;
>   struct mtk_mipi_tx *mipi_tx;
>   struct resource *mem;
> - struct clk *ref_clk;
>   const char *ref_clk_name;
>   struct clk_init_data clk_init = {
> - .ops = &mtk_mipi_tx_pll_ops,
>   .num_parents = 1,
>   .parent_names = (const char * const *)&ref_clk_name,
>   .flags = CLK_SET_RATE_GATE,
> @@ -408,6 +132,7 @@ static int mtk_mipi_tx_probe(struct platform_device *pdev)
>   return -ENOMEM;
>  
>   mipi_tx->driver_data = of_device_get_match_data(dev);
> +
>   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   mipi_tx->regs = devm_ioremap_resource(dev, mem);
>   if (IS_ERR(mipi_tx->regs)) {
> @@ -416,13 +141,14 @@ static int mtk_mipi_tx_probe(struct platform_device 
> *pdev)
>   return ret;
>   }
>  
> - ref_clk = devm_clk_get(dev, NULL);
> - if (IS_ERR(ref_clk)) {
> - ret = PTR_ERR(ref_clk);
> + mipi_tx->ref_clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(mipi_tx->ref_clk)) {
> + ret = PTR_ERR(mipi_tx->ref_clk);
>   dev_err(dev, "Failed to get reference clock: %d\n", ret);
>   return ret;
>   }
> - ref_clk_name = __clk_get_name(ref_clk);
> +
> + ref_clk_name = __clk_get_name(mipi_tx->ref_clk);
>  
>   ret = of_property_read_string(dev->of_node, "clock-output-names",
> &clk_init.name);
> @@ -431,6 +157,8 @@ static int mtk_mipi_tx_probe(struct platform_device *pdev)
>   return ret;
>   }
>  
> + mtk_mipi_tx_clk_get_ops(mipi_tx, &clk_init.ops);
> +
>   mipi_tx->pll_hw.init = &clk_init;
>   mipi_tx->pll = devm_clk_register(dev, &mipi_tx->pll_hw);
>   if (IS_ERR(mipi_tx->pll)) {
> @@ -465,20 +193,12 @@ static int mtk_mipi_tx_remove(struct platform_device 
> *pdev)
>   return 0;
>  }
>  
> -static const struct mtk_mipitx_data mt2701_mipitx_data = {
> - .mppll_preserve = (3 << 8)
> -};
> -
> -static const struct mtk_mipitx_data mt8173_mipitx_data = {
> - .mppll_preserve = (0 << 8)
> -};
> -
>  static const struct of_device_id mtk_mipi_tx_match[] = {
>   { .compatible = "mediatek,mt2701-mipi-tx",
> .data = &mt2701_mipitx_data },
>   { .compatible = "mediatek,mt8173-mipi-tx",
> .data = &mt8173_mipitx_data },
> - {},
> + { },
>  };
>  





[PATCH v1 1/2] drm/mediatek: separate mipi_tx to different file

2019-02-19 Thread Jitao Shi
Different IC has different mipi_tx setting of dsi.
This patch separates the mipi_tx hardware relate part for mt8173.

Signed-off-by: Jitao Shi 
---
 drivers/gpu/drm/mediatek/Makefile |   1 +
 drivers/gpu/drm/mediatek/mtk_mipi_tx.c| 350 ++
 drivers/gpu/drm/mediatek/mtk_mipi_tx.h|  51 +++
 drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c | 290 +++
 4 files changed, 377 insertions(+), 315 deletions(-)
 create mode 100644 drivers/gpu/drm/mediatek/mtk_mipi_tx.h
 create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c

diff --git a/drivers/gpu/drm/mediatek/Makefile 
b/drivers/gpu/drm/mediatek/Makefile
index 82ae49c64221..2c8de1f5a5ee 100644
--- a/drivers/gpu/drm/mediatek/Makefile
+++ b/drivers/gpu/drm/mediatek/Makefile
@@ -12,6 +12,7 @@ mediatek-drm-y := mtk_disp_color.o \
  mtk_drm_plane.o \
  mtk_dsi.o \
  mtk_mipi_tx.o \
+ mtk_mt8173_mipi_tx.o \
  mtk_dpi.o
 
 obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o
diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c 
b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
index 90e913108950..fa361c8be8d7 100644
--- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
+++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
@@ -11,292 +11,45 @@
  * GNU General Public License for more details.
  */
 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#define MIPITX_DSI_CON 0x00
-#define RG_DSI_LDOCORE_EN  BIT(0)
-#define RG_DSI_CKG_LDOOUT_EN   BIT(1)
-#define RG_DSI_BCLK_SEL(3 << 2)
-#define RG_DSI_LD_IDX_SEL  (7 << 4)
-#define RG_DSI_PHYCLK_SEL  (2 << 8)
-#define RG_DSI_DSICLK_FREQ_SEL BIT(10)
-#define RG_DSI_LPTX_CLMP_ENBIT(11)
-
-#define MIPITX_DSI_CLOCK_LANE  0x04
-#define MIPITX_DSI_DATA_LANE0  0x08
-#define MIPITX_DSI_DATA_LANE1  0x0c
-#define MIPITX_DSI_DATA_LANE2  0x10
-#define MIPITX_DSI_DATA_LANE3  0x14
-#define RG_DSI_LNTx_LDOOUT_EN  BIT(0)
-#define RG_DSI_LNTx_CKLANE_EN  BIT(1)
-#define RG_DSI_LNTx_LPTX_IPLUS1BIT(2)
-#define RG_DSI_LNTx_LPTX_IPLUS2BIT(3)
-#define RG_DSI_LNTx_LPTX_IMINUSBIT(4)
-#define RG_DSI_LNTx_LPCD_IPLUS BIT(5)
-#define RG_DSI_LNTx_LPCD_IMINUSBIT(6)
-#define RG_DSI_LNTx_RT_CODE(0xf << 8)
-
-#define MIPITX_DSI_TOP_CON 0x40
-#define RG_DSI_LNT_INTR_EN BIT(0)
-#define RG_DSI_LNT_HS_BIAS_EN  BIT(1)
-#define RG_DSI_LNT_IMP_CAL_EN  BIT(2)
-#define RG_DSI_LNT_TESTMODE_EN BIT(3)
-#define RG_DSI_LNT_IMP_CAL_CODE(0xf << 4)
-#define RG_DSI_LNT_AIO_SEL (7 << 8)
-#define RG_DSI_PAD_TIE_LOW_EN  BIT(11)
-#define RG_DSI_DEBUG_INPUT_EN  BIT(12)
-#define RG_DSI_PRESERVE(7 << 13)
-
-#define MIPITX_DSI_BG_CON  0x44
-#define RG_DSI_BG_CORE_EN  BIT(0)
-#define RG_DSI_BG_CKEN BIT(1)
-#define RG_DSI_BG_DIV  (0x3 << 2)
-#define RG_DSI_BG_FAST_CHARGE  BIT(4)
-#define RG_DSI_VOUT_MSK(0x3 << 5)
-#define RG_DSI_V12_SEL (7 << 5)
-#define RG_DSI_V10_SEL (7 << 8)
-#define RG_DSI_V072_SEL(7 << 11)
-#define RG_DSI_V04_SEL (7 << 14)
-#define RG_DSI_V032_SEL(7 << 17)
-#define RG_DSI_V02_SEL (7 << 20)
-#define RG_DSI_BG_R1_TRIM  (0xf << 24)
-#define RG_DSI_BG_R2_TRIM  (0xf << 28)
-
-#define MIPITX_DSI_PLL_CON00x50
-#define RG_DSI_MPPLL_PLL_ENBIT(0)
-#define RG_DSI_MPPLL_DIV_MSK   (0x1ff << 1)
-#define RG_DSI_MPPLL_PREDIV(3 << 1)
-#define RG_DSI_MPPLL_TXDIV0(3 << 3)
-#define RG_DSI_MPPLL_TXDIV1(3 << 5)
-#define RG_DSI_MPPLL_POSDIV(7 << 7)
-#define RG_DSI_MPPLL_MONVC_EN  BIT(10)
-#define RG_DSI_MPPLL_MONREF_EN BIT(11)
-#define RG_DSI_MPPLL_VOD_ENBIT(12)
-
-#define MIPITX_DSI_PLL_CON10x54
-#define RG_DSI_MPPLL_SDM_FRA_ENBIT(0)
-#define RG_DSI_MPPLL_SDM_SSC_PH_INIT   BIT(1)
-#define RG_DSI_MPPLL_SDM_SSC_ENBIT(2)
-#define RG_DSI_MPPLL_SDM_SSC_PRD   (0x << 16)
-
-#define MIPITX_DSI_PLL_CON20x58
-
-#define MIPITX_DSI_PLL_TOP 0x64
-#define RG_DSI_MPPLL_PRESERVE  (0xff << 8)
-
-#define MIPITX_DSI_PLL_PWR 0x68
-#define RG_DSI_MPPLL_SDM_PWR_ONBIT(0)
-#define RG_DSI_MPPLL_SDM_ISO_ENBIT(1)
-#define RG_DSI_MPPLL_SDM_PWR_ACK   BIT(8)
-
-#define MIPITX_DSI_SW_CTRL 0x80
-#define SW_CTRL_EN BIT(0)
-
-#define MIPITX_DSI_SW_CTRL_CON00x84
-#define SW_LNTC_LPTX_PRE_OEBIT(0)
-#define SW_LNTC_LPTX_OEBIT(1)
-#define SW_LNTC_LPTX_P BIT(2)
-#define SW_LNTC_LPTX_N