From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4348EC32771 for ; Tue, 7 Jan 2020 01:12:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id EE0952072C for ; Tue, 7 Jan 2020 01:12:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="RQEemnC4" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727388AbgAGBMA (ORCPT ); Mon, 6 Jan 2020 20:12:00 -0500 Received: from perceval.ideasonboard.com ([213.167.242.64]:42644 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727250AbgAGBL7 (ORCPT ); Mon, 6 Jan 2020 20:11:59 -0500 Received: from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6A37252F; Tue, 7 Jan 2020 02:11:55 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1578359515; bh=Tkc9qc4Jjwmob2OzCBqN9N+b0u1yHJN64Y5ORbvsC4w=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=RQEemnC4/8f6EVdwz+0yQbKl04xmGmn2VUykKUtW6xXzeHezyKZaUrmZpBiizoOMc RZUV4/E331jGcxT9ro8gPqiKE++xiMxC1YGIUIuTDxtcLCfHzFf8xh7k+FOCEgFcZq wpQO6vjtUP/k+vubf7onuen94RU+y9K1peiAZ+Zw= Date: Tue, 7 Jan 2020 03:11:44 +0200 From: Laurent Pinchart To: Helen Koike Cc: linux-rockchip@lists.infradead.org, mark.rutland@arm.com, devicetree@vger.kernel.org, eddie.cai.linux@gmail.com, mchehab@kernel.org, heiko@sntech.de, gregkh@linuxfoundation.org, andrey.konovalov@linaro.org, linux-kernel@vger.kernel.org, tfiga@chromium.org, robh+dt@kernel.org, hans.verkuil@cisco.com, sakari.ailus@linux.intel.com, joacim.zetterling@gmail.com, kernel@collabora.com, ezequiel@collabora.com, linux-media@vger.kernel.org, jacob-chen@iotwrt.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v12 01/11] media: staging: phy-rockchip-dphy: add Rockchip MIPI Synopsys DPHY driver Message-ID: <20200107011144.GB28230@pendragon.ideasonboard.com> References: <20191227200116.2612137-1-helen.koike@collabora.com> <20191227200116.2612137-2-helen.koike@collabora.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20191227200116.2612137-2-helen.koike@collabora.com> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Helen and Ezequiel, Thank you for the patch. On Fri, Dec 27, 2019 at 05:01:06PM -0300, Helen Koike wrote: > From: Ezequiel Garcia > > Add driver for Rockchip MIPI Synopsys DPHY driver > > Signed-off-by: Tomasz Figa > Signed-off-by: Ezequiel Garcia > Signed-off-by: Helen Koike > > --- > > Changes in v12: > - several cleanups > - remove "rx" from function names, as this driver only supports rx > > Changes in v11: > - fix checkpatch errors > > Changes in v10: None > Changes in v9: > - Move to staging > - replace memcpy by a directly assignment > - remove unecessary ret variable in rockchip_dphy_init > - s/0x1/1 > - s/0x0/0 > - coding style changes > - dphy_reg variable sizes > - variables from int to unsigned int > - rename functions to start with rk_ > - rename dphy0 to rx > - fix hardcoded lane0 usage > - disable rx on power off > - general cleanups of unused variables > > Changes in v8: > - Remove boiler plate license text > > Changes in v7: > - Migrate dphy specific code from > drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c > to drivers/phy/rockchip/phy-rockchip-dphy.c > - Drop support for rk3288 > - Drop support for dphy txrx > - code styling and checkpatch fixes > > drivers/staging/media/Kconfig | 2 + > drivers/staging/media/Makefile | 1 + > .../staging/media/phy-rockchip-dphy/Kconfig | 11 + > .../staging/media/phy-rockchip-dphy/Makefile | 2 + > drivers/staging/media/phy-rockchip-dphy/TODO | 6 + > .../phy-rockchip-dphy/phy-rockchip-dphy.c | 396 ++++++++++++++++++ > 6 files changed, 418 insertions(+) > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Kconfig > create mode 100644 drivers/staging/media/phy-rockchip-dphy/Makefile > create mode 100644 drivers/staging/media/phy-rockchip-dphy/TODO > create mode 100644 drivers/staging/media/phy-rockchip-dphy/phy-rockchip-dphy.c > > diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig > index 642adc4c24d2..a47484473883 100644 > --- a/drivers/staging/media/Kconfig > +++ b/drivers/staging/media/Kconfig > @@ -38,4 +38,6 @@ source "drivers/staging/media/ipu3/Kconfig" > > source "drivers/staging/media/soc_camera/Kconfig" > > +source "drivers/staging/media/phy-rockchip-dphy/Kconfig" > + > endif > diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile > index 2f1711a8aeed..b0eae3906208 100644 > --- a/drivers/staging/media/Makefile > +++ b/drivers/staging/media/Makefile > @@ -8,3 +8,4 @@ obj-$(CONFIG_TEGRA_VDE) += tegra-vde/ > obj-$(CONFIG_VIDEO_HANTRO) += hantro/ > obj-$(CONFIG_VIDEO_IPU3_IMGU) += ipu3/ > obj-$(CONFIG_SOC_CAMERA) += soc_camera/ > +obj-$(CONFIG_PHY_ROCKCHIP_DPHY) += phy-rockchip-dphy/ > diff --git a/drivers/staging/media/phy-rockchip-dphy/Kconfig b/drivers/staging/media/phy-rockchip-dphy/Kconfig > new file mode 100644 > index 000000000000..7378bd75fa7c > --- /dev/null > +++ b/drivers/staging/media/phy-rockchip-dphy/Kconfig > @@ -0,0 +1,11 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# Phy drivers for Rockchip platforms s/Phy/MIPI D-PHY/ > +# > +config PHY_ROCKCHIP_DPHY > + tristate "Rockchip MIPI Synopsys DPHY driver" > + depends on (ARCH_ROCKCHIP || COMPILE_TEST) && OF > + select GENERIC_PHY_MIPI_DPHY > + select GENERIC_PHY > + help > + Enable this to support the Rockchip MIPI Synopsys DPHY. > diff --git a/drivers/staging/media/phy-rockchip-dphy/Makefile b/drivers/staging/media/phy-rockchip-dphy/Makefile > new file mode 100644 > index 000000000000..24679dc950cd > --- /dev/null > +++ b/drivers/staging/media/phy-rockchip-dphy/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0 > +obj-$(CONFIG_PHY_ROCKCHIP_DPHY) += phy-rockchip-dphy.o > diff --git a/drivers/staging/media/phy-rockchip-dphy/TODO b/drivers/staging/media/phy-rockchip-dphy/TODO > new file mode 100644 > index 000000000000..e1fda55babef > --- /dev/null > +++ b/drivers/staging/media/phy-rockchip-dphy/TODO > @@ -0,0 +1,6 @@ > +The major reason for keeping this in staging is because the only driver s/major/main/ > +who uses this is rkisp1 who is also in staging. It should be moved together s/who uses this/that uses this/ s/who is also/, which is also/ > +rkisp1. s/rkisp1/with rkisp1/ ? > + > +Please CC patches to Linux Media and > +Helen Koike . > diff --git a/drivers/staging/media/phy-rockchip-dphy/phy-rockchip-dphy.c b/drivers/staging/media/phy-rockchip-dphy/phy-rockchip-dphy.c > new file mode 100644 > index 000000000000..c3fe9c64b45f > --- /dev/null > +++ b/drivers/staging/media/phy-rockchip-dphy/phy-rockchip-dphy.c > @@ -0,0 +1,396 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) Any reason for this ? Kernel code is usually GPL-2.0. > +/* > + * Rockchip MIPI Synopsys DPHY driver > + * > + * Copyright (C) 2019 Collabora, Ltd. > + * > + * Based on: > + * > + * drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c > + * in https://chromium.googlesource.com/chromiumos/third_party/kernel, > + * chromeos-4.4 branch. > + * > + * Copyright (C) 2017 Fuzhou Rockchip Electronics Co., Ltd. > + * Jacob Chen > + * Shunqian Zheng > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define RK3399_GRF_SOC_CON9 0x6224 > +#define RK3399_GRF_SOC_CON21 0x6254 > +#define RK3399_GRF_SOC_CON22 0x6258 > +#define RK3399_GRF_SOC_CON23 0x625c > +#define RK3399_GRF_SOC_CON24 0x6260 > +#define RK3399_GRF_SOC_CON25 0x6264 > +#define RK3399_GRF_SOC_STATUS1 0xe2a4 > + > +#define CLOCK_LANE_HS_RX_CONTROL 0x34 > +#define LANE0_HS_RX_CONTROL 0x44 > +#define LANE1_HS_RX_CONTROL 0x54 > +#define LANE2_HS_RX_CONTROL 0x84 > +#define LANE3_HS_RX_CONTROL 0x94 > +#define LANES_THS_SETTLE_CONTROL 0x75 > +#define THS_SETTLE_COUNTER_THRESHOLD 0x04 > + > +struct hsfreq_range { > + u16 range_h; > + u8 cfg_bit; > +}; > + > +static const struct hsfreq_range rk3399_mipidphy_hsfreq_ranges[] = { > + { 89, 0x00 }, { 99, 0x10 }, { 109, 0x20 }, { 129, 0x01 }, > + { 139, 0x11 }, { 149, 0x21 }, { 169, 0x02 }, { 179, 0x12 }, > + { 199, 0x22 }, { 219, 0x03 }, { 239, 0x13 }, { 249, 0x23 }, > + { 269, 0x04 }, { 299, 0x14 }, { 329, 0x05 }, { 359, 0x15 }, > + { 399, 0x25 }, { 449, 0x06 }, { 499, 0x16 }, { 549, 0x07 }, > + { 599, 0x17 }, { 649, 0x08 }, { 699, 0x18 }, { 749, 0x09 }, > + { 799, 0x19 }, { 849, 0x29 }, { 899, 0x39 }, { 949, 0x0a }, > + { 999, 0x1a }, { 1049, 0x2a }, { 1099, 0x3a }, { 1149, 0x0b }, > + { 1199, 0x1b }, { 1249, 0x2b }, { 1299, 0x3b }, { 1349, 0x0c }, > + { 1399, 0x1c }, { 1449, 0x2c }, { 1500, 0x3c } > +}; > + > +static const char * const rk3399_mipidphy_clks[] = { > + "dphy-ref", > + "dphy-cfg", > + "grf", > +}; > + > +enum dphy_reg_id { > + GRF_DPHY_RX0_TURNDISABLE = 0, > + GRF_DPHY_RX0_FORCERXMODE, > + GRF_DPHY_RX0_FORCETXSTOPMODE, > + GRF_DPHY_RX0_ENABLE, > + GRF_DPHY_RX0_TESTCLR, > + GRF_DPHY_RX0_TESTCLK, > + GRF_DPHY_RX0_TESTEN, > + GRF_DPHY_RX0_TESTDIN, > + GRF_DPHY_RX0_TURNREQUEST, > + GRF_DPHY_RX0_TESTDOUT, > + GRF_DPHY_TX0_TURNDISABLE, > + GRF_DPHY_TX0_FORCERXMODE, > + GRF_DPHY_TX0_FORCETXSTOPMODE, > + GRF_DPHY_TX0_TURNREQUEST, > + GRF_DPHY_TX1RX1_TURNDISABLE, > + GRF_DPHY_TX1RX1_FORCERXMODE, > + GRF_DPHY_TX1RX1_FORCETXSTOPMODE, > + GRF_DPHY_TX1RX1_ENABLE, > + GRF_DPHY_TX1RX1_MASTERSLAVEZ, > + GRF_DPHY_TX1RX1_BASEDIR, > + GRF_DPHY_TX1RX1_ENABLECLK, > + GRF_DPHY_TX1RX1_TURNREQUEST, > + GRF_DPHY_RX1_SRC_SEL, > + /* rk3288 only */ > + GRF_CON_DISABLE_ISP, > + GRF_CON_ISP_DPHY_SEL, > + GRF_DSI_CSI_TESTBUS_SEL, > + GRF_DVP_V18SEL, > + /* below is for rk3399 only */ > + GRF_DPHY_RX0_CLK_INV_SEL, > + GRF_DPHY_RX1_CLK_INV_SEL, > +}; > + > +struct dphy_reg { > + u16 offset; > + u8 mask; > + u8 shift; > +}; > + > +#define PHY_REG(_offset, _width, _shift) \ > + { .offset = _offset, .mask = BIT(_width) - 1, .shift = _shift, } > + > +static const struct dphy_reg rk3399_grf_dphy_regs[] = { > + [GRF_DPHY_RX0_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON9, 4, 0), > + [GRF_DPHY_RX0_CLK_INV_SEL] = PHY_REG(RK3399_GRF_SOC_CON9, 1, 10), > + [GRF_DPHY_RX1_CLK_INV_SEL] = PHY_REG(RK3399_GRF_SOC_CON9, 1, 11), > + [GRF_DPHY_RX0_ENABLE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 0), > + [GRF_DPHY_RX0_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 4), > + [GRF_DPHY_RX0_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 8), > + [GRF_DPHY_RX0_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 12), > + [GRF_DPHY_TX0_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 0), > + [GRF_DPHY_TX0_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 4), > + [GRF_DPHY_TX0_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 8), > + [GRF_DPHY_TX0_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 12), > + [GRF_DPHY_TX1RX1_ENABLE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 0), > + [GRF_DPHY_TX1RX1_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 4), > + [GRF_DPHY_TX1RX1_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 8), > + [GRF_DPHY_TX1RX1_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 12), > + [GRF_DPHY_TX1RX1_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON24, 4, 0), > + [GRF_DPHY_RX1_SRC_SEL] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 4), > + [GRF_DPHY_TX1RX1_BASEDIR] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 5), > + [GRF_DPHY_TX1RX1_ENABLECLK] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 6), > + [GRF_DPHY_TX1RX1_MASTERSLAVEZ] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 7), > + [GRF_DPHY_RX0_TESTDIN] = PHY_REG(RK3399_GRF_SOC_CON25, 8, 0), > + [GRF_DPHY_RX0_TESTEN] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 8), > + [GRF_DPHY_RX0_TESTCLK] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 9), > + [GRF_DPHY_RX0_TESTCLR] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 10), > + [GRF_DPHY_RX0_TESTDOUT] = PHY_REG(RK3399_GRF_SOC_STATUS1, 8, 0), > +}; > + > +struct rk_dphy_drv_data { > + const char * const *clks; > + unsigned int num_clks; > + const struct hsfreq_range *hsfreq_ranges; > + unsigned int num_hsfreq_ranges; > + const struct dphy_reg *regs; > +}; > + > +struct rk_dphy { > + struct device *dev; > + struct regmap *grf; > + struct clk_bulk_data *clks; > + > + const struct rk_dphy_drv_data *drv_data; > + struct phy_configure_opts_mipi_dphy config; > + > + u8 hsfreq; > +}; > + > +static inline void rk_dphy_write_grf(struct rk_dphy *priv, > + unsigned int index, u8 value) > +{ > + const struct dphy_reg *reg = &priv->drv_data->regs[index]; > + /* Update high word */ > + unsigned int val = (value << reg->shift) | > + (reg->mask << (reg->shift + 16)); > + > + WARN_ON(!reg->offset); Maybe if (WARN_ON(!reg->offset)) return; > + regmap_write(priv->grf, reg->offset, val); > +} > + > +static void rk_dphy_write(struct rk_dphy *priv, > + u8 test_code, u8 test_data) > +{ > + /* > + * With the falling edge on TESTCLK, the TESTDIN[7:0] signal content > + * is latched internally as the current test code. Test data is > + * programmed internally by rising edge on TESTCLK. > + */ > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_TESTCLK, 1); Do we need this first line, as the function always exits with TESTCLK=1, and TESTCLK is initialized to 1 before calling rk_dphy_write for the first time ? > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_TESTDIN, test_code); > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_TESTEN, 1); > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_TESTCLK, 0); > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_TESTEN, 0); > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_TESTDIN, test_data); > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_TESTCLK, 1); > +} > + > +static void rk_dphy_enable(struct rk_dphy *priv) > +{ > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_FORCERXMODE, 0); > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_FORCETXSTOPMODE, 0); > + > + /* Disable lane turn around, which is ignored in receive mode */ > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_TURNREQUEST, 0); > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_TURNDISABLE, 0xf); > + > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_ENABLE, > + GENMASK(priv->config.lanes - 1, 0)); > + > + /* dphy start */ > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_TESTCLK, 1); > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_TESTCLR, 1); > + usleep_range(100, 150); > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_TESTCLR, 0); > + usleep_range(100, 150); > + > + /* set clock lane */ > + /* HS hsfreq_range & lane 0 settle bypass */ > + rk_dphy_write(priv, CLOCK_LANE_HS_RX_CONTROL, 0); > + /* HS RX Control of lane0 */ > + rk_dphy_write(priv, LANE0_HS_RX_CONTROL, priv->hsfreq << 1); > + /* HS RX Control of lane1 */ > + rk_dphy_write(priv, LANE1_HS_RX_CONTROL, priv->hsfreq << 1); > + /* HS RX Control of lane2 */ > + rk_dphy_write(priv, LANE2_HS_RX_CONTROL, priv->hsfreq << 1); > + /* HS RX Control of lane3 */ > + rk_dphy_write(priv, LANE3_HS_RX_CONTROL, priv->hsfreq << 1); > + /* HS RX Data Lanes Settle State Time Control */ > + rk_dphy_write(priv, LANES_THS_SETTLE_CONTROL, > + THS_SETTLE_COUNTER_THRESHOLD); > + > + /* Normal operation */ > + rk_dphy_write(priv, 0x0, 0); > +} > + > +static int rk_dphy_configure(struct phy *phy, union phy_configure_opts *opts) > +{ > + struct rk_dphy *priv = phy_get_drvdata(phy); > + const struct rk_dphy_drv_data *drv_data = priv->drv_data; > + struct phy_configure_opts_mipi_dphy *config = &opts->mipi_dphy; > + unsigned int i, hsfreq = 0, data_rate_mbps = config->hs_clk_rate; Maybe one variable per line ? > + int ret; > + > + /* pass with phy_mipi_dphy_get_default_config (with pixel rate?) */ > + ret = phy_mipi_dphy_config_validate(config); > + if (ret) > + return ret; I would add a blank line here. > + do_div(data_rate_mbps, 1000 * 1000); data_rate_mbps is an unsigned int, so you can use data_rate_mbps /= 1000 * 1000; However, you're potentially truncating hs_clk_rate by assigning it to data_rate_mbps, so I would remove the assignment at declaration time and do data_rate_mbps = div_u64(config->hs_clk_rate, 1000 * 1000); or define data_rate_mbps as an unsigned long. > + > + dev_dbg(priv->dev, "lanes %d - data_rate_mbps %u\n", > + config->lanes, data_rate_mbps); > + for (i = 0; i < drv_data->num_hsfreq_ranges; i++) { > + if (drv_data->hsfreq_ranges[i].range_h >= data_rate_mbps) { > + hsfreq = drv_data->hsfreq_ranges[i].cfg_bit; > + break; > + } > + } > + if (!hsfreq) > + return -EINVAL; > + > + priv->hsfreq = hsfreq; > + priv->config = *config; > + return 0; > +} > + > +static int rk_dphy_power_on(struct phy *phy) > +{ > + struct rk_dphy *priv = phy_get_drvdata(phy); > + int ret; > + > + ret = clk_bulk_enable(priv->drv_data->num_clks, priv->clks); > + if (ret) > + return ret; > + > + rk_dphy_enable(priv); > + > + return 0; > +} > + > +static int rk_dphy_power_off(struct phy *phy) > +{ > + struct rk_dphy *priv = phy_get_drvdata(phy); > + > + rk_dphy_write_grf(priv, GRF_DPHY_RX0_ENABLE, 0); > + clk_bulk_disable(priv->drv_data->num_clks, priv->clks); > + return 0; > +} > + > +static int rk_dphy_init(struct phy *phy) > +{ > + struct rk_dphy *priv = phy_get_drvdata(phy); > + > + return clk_bulk_prepare(priv->drv_data->num_clks, priv->clks); > +} > + > +static int rk_dphy_exit(struct phy *phy) > +{ > + struct rk_dphy *priv = phy_get_drvdata(phy); > + > + clk_bulk_unprepare(priv->drv_data->num_clks, priv->clks); > + return 0; > +} > + > +static const struct phy_ops rk_dphy_ops = { > + .power_on = rk_dphy_power_on, > + .power_off = rk_dphy_power_off, > + .init = rk_dphy_init, > + .exit = rk_dphy_exit, > + .configure = rk_dphy_configure, > + .owner = THIS_MODULE, > +}; > + > +static const struct rk_dphy_drv_data rk3399_mipidphy_drv_data = { > + .clks = rk3399_mipidphy_clks, > + .num_clks = ARRAY_SIZE(rk3399_mipidphy_clks), > + .hsfreq_ranges = rk3399_mipidphy_hsfreq_ranges, > + .num_hsfreq_ranges = ARRAY_SIZE(rk3399_mipidphy_hsfreq_ranges), > + .regs = rk3399_grf_dphy_regs, > +}; > + > +static const struct of_device_id rk_dphy_dt_ids[] = { > + { > + .compatible = "rockchip,rk3399-mipi-dphy", > + .data = &rk3399_mipidphy_drv_data, > + }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, rk_dphy_dt_ids); > + > +static int rk_dphy_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + const struct rk_dphy_drv_data *drv_data; > + struct phy_provider *phy_provider; > + const struct of_device_id *of_id; > + struct rk_dphy *priv; > + struct regmap *grf; > + struct phy *phy; > + unsigned int i; > + int ret; > + > + if (!dev->parent || !dev->parent->of_node) > + return -ENODEV; > + > + if (platform_get_resource(pdev, IORESOURCE_MEM, 0)) { > + dev_err(dev, "Rockchip DPHY driver only suports RX mode\n"); > + return -EINVAL; > + } > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + priv->dev = dev; > + > + grf = syscon_node_to_regmap(dev->parent->of_node); > + if (IS_ERR(grf)) { > + grf = syscon_regmap_lookup_by_phandle(dev->of_node, > + "rockchip,grf"); Is this for backward compatibility with older bindings ? Do we still need it ? If so I would add a comment to explain why. With the above small issues fixed, Reviewed-by: Laurent Pinchart > + if (IS_ERR(grf)) { > + dev_err(dev, "Can't find GRF syscon\n"); > + return -ENODEV; > + } > + } > + priv->grf = grf; > + > + of_id = of_match_device(rk_dphy_dt_ids, dev); > + if (!of_id) > + return -EINVAL; > + > + drv_data = of_id->data; > + priv->drv_data = drv_data; > + priv->clks = devm_kcalloc(&pdev->dev, drv_data->num_clks, > + sizeof(*priv->clks), GFP_KERNEL); > + if (!priv->clks) > + return -ENOMEM; > + for (i = 0; i < drv_data->num_clks; i++) > + priv->clks[i].id = drv_data->clks[i]; > + ret = devm_clk_bulk_get(&pdev->dev, drv_data->num_clks, priv->clks); > + if (ret) > + return ret; > + > + phy = devm_phy_create(dev, np, &rk_dphy_ops); > + if (IS_ERR(phy)) { > + dev_err(dev, "failed to create phy\n"); > + return PTR_ERR(phy); > + } > + phy_set_drvdata(phy, priv); > + > + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); > + > + return PTR_ERR_OR_ZERO(phy_provider); > +} > + > +static struct platform_driver rk_dphy_driver = { > + .probe = rk_dphy_probe, > + .driver = { > + .name = "rockchip-mipi-dphy", > + .of_match_table = rk_dphy_dt_ids, > + }, > +}; > +module_platform_driver(rk_dphy_driver); > + > +MODULE_AUTHOR("Ezequiel Garcia "); > +MODULE_DESCRIPTION("Rockchip MIPI Synopsys DPHY driver"); > +MODULE_LICENSE("Dual MIT/GPL"); -- Regards, Laurent Pinchart