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=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED 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 2F280C43334 for ; Wed, 5 Sep 2018 13:46:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D2DF82075C for ; Wed, 5 Sep 2018 13:46:04 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="soNsk45r" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D2DF82075C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727681AbeIESQV (ORCPT ); Wed, 5 Sep 2018 14:16:21 -0400 Received: from perceval.ideasonboard.com ([213.167.242.64]:57058 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726008AbeIESQU (ORCPT ); Wed, 5 Sep 2018 14:16:20 -0400 Received: from avalon.localnet (unknown [IPv6:2a02:a03f:4407:cd00:1953:677d:5909:a7be]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3194884; Wed, 5 Sep 2018 15:45:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1536155159; bh=zNtiViidA4HMSm8KX4y/8iXT+TKbi0B1VlaG9hASk70=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=soNsk45rnSvvFYQmV5HlSo64QcKQTSBcNMkQT0gV92qhJHrIP/kcoPtSMaF0fPqbi dQ78xUlcL7q7O4JGnqYpTxdl9ONenc5aR0oUhVyZPC9V97x8YdmOcphUDZvq04L1e7 O+QvwWR+J3/9VTWdeNJJEwobSaky9RMJR58xwh9A= From: Laurent Pinchart To: Maxime Ripard Cc: Kishon Vijay Abraham I , Boris Brezillon , Thomas Petazzoni , linux-media@vger.kernel.org, Archit Taneja , Andrzej Hajda , Chen-Yu Tsai , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, Krzysztof Witos , Rafal Ciepiela Subject: Re: [PATCH 04/10] phy: dphy: Add configuration helpers Date: Wed, 05 Sep 2018 16:46:05 +0300 Message-ID: <3617916.Vq2Smf1hnZ@avalon> Organization: Ideas on Board Oy In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Maxime, Thank you for the patch. On Wednesday, 5 September 2018 12:16:35 EEST Maxime Ripard wrote: > The MIPI D-PHY spec defines default values and boundaries for most of the > parameters it defines. Introduce helpers to help drivers get meaningful > values based on their current parameters, and validate the boundaries of > these parameters if needed. > > Signed-off-by: Maxime Ripard > --- > drivers/phy/Kconfig | 8 ++- > drivers/phy/Makefile | 1 +- > drivers/phy/phy-core-mipi-dphy.c | 160 +++++++++++++++++++++++++++++++- > include/linux/phy/phy-mipi-dphy.h | 6 +- > 4 files changed, 175 insertions(+) > create mode 100644 drivers/phy/phy-core-mipi-dphy.c > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index 5c8d452e35e2..06bd22bd1f4a 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -15,6 +15,14 @@ config GENERIC_PHY > phy users can obtain reference to the PHY. All the users of this > framework should select this config. > > +config GENERIC_PHY_MIPI_DPHY > + bool "MIPI D-PHY support" > + help > + Generic MIPI D-PHY support. > + > + Provides a number of helpers a core functions for MIPI D-PHY > + drivers to us. Do we really need to make this user-selectable ? > config PHY_LPC18XX_USB_OTG > tristate "NXP LPC18xx/43xx SoC USB OTG PHY driver" > depends on OF && (ARCH_LPC18XX || COMPILE_TEST) > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile > index 84e3bd9c5665..71c29d2b9af7 100644 > --- a/drivers/phy/Makefile > +++ b/drivers/phy/Makefile > @@ -4,6 +4,7 @@ > # > > obj-$(CONFIG_GENERIC_PHY) += phy-core.o > +obj-$(CONFIG_GENERIC_PHY_MIPI_DPHY) += phy-core-mipi-dphy.o > obj-$(CONFIG_PHY_LPC18XX_USB_OTG) += phy-lpc18xx-usb-otg.o > obj-$(CONFIG_PHY_XGENE) += phy-xgene.o > obj-$(CONFIG_PHY_PISTACHIO_USB) += phy-pistachio-usb.o > diff --git a/drivers/phy/phy-core-mipi-dphy.c > b/drivers/phy/phy-core-mipi-dphy.c new file mode 100644 > index 000000000000..6c1ddc7734a2 > --- /dev/null > +++ b/drivers/phy/phy-core-mipi-dphy.c > @@ -0,0 +1,160 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2013 NVIDIA Corporation > + * Copyright (C) 2018 Cadence Design Systems Inc. > + */ > + > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +/* > + * Default D-PHY timings based on MIPI D-PHY specification. Derived from > the > + * valid ranges specified in Section 6.9, Table 14, Page 40 of the D-PHY > + * specification (v1.2) with minor adjustments. Could you list those adjustments ? > + */ > +int phy_mipi_dphy_get_default_config(unsigned long pixel_clock, > + unsigned int bpp, > + unsigned int lanes, > + struct phy_configure_opts_mipi_dphy *cfg) > +{ > + unsigned long hs_clk_rate; > + unsigned long ui; > + > + if (!cfg) > + return -EINVAL; Should we really expect cfg to be NULL ? > + hs_clk_rate = pixel_clock * bpp / lanes; > + ui = DIV_ROUND_UP(NSEC_PER_SEC, hs_clk_rate); > + > + cfg->clk_miss = 0; > + cfg->clk_post = 70 + 52 * ui; > + cfg->clk_pre = 8; > + cfg->clk_prepare = 65; > + cfg->clk_settle = 95; > + cfg->clk_term_en = 0; > + cfg->clk_trail = 80; > + cfg->clk_zero = 260; > + cfg->d_term_en = 0; > + cfg->eot = 0; > + cfg->hs_exit = 120; > + cfg->hs_prepare = 65 + 5 * ui; > + cfg->hs_zero = 145 + 5 * ui; > + cfg->hs_settle = 85 + 6 * ui; > + cfg->hs_skip = 40; > + > + /* > + * The MIPI D-PHY specification (Section 6.9, v1.2, Table 14, Page 40) > + * contains this formula as: > + * > + * T_HS-TRAIL = max(n * 8 * ui, 60 + n * 4 * ui) > + * > + * where n = 1 for forward-direction HS mode and n = 4 for reverse- > + * direction HS mode. There's only one setting and this function does > + * not parameterize on anything other that ui, so this code will > + * assumes that reverse-direction HS mode is supported and uses n = 4. > + */ > + cfg->hs_trail = max(4 * 8 * ui, 60 + 4 * 4 * ui); > + > + cfg->init = 100000; > + cfg->lpx = 60; > + cfg->ta_get = 5 * cfg->lpx; > + cfg->ta_go = 4 * cfg->lpx; > + cfg->ta_sure = 2 * cfg->lpx; > + cfg->wakeup = 1000000; > + > + cfg->hs_clk_rate = hs_clk_rate; > + cfg->lanes = lanes; > + > + return 0; > +} > +EXPORT_SYMBOL(phy_mipi_dphy_get_default_config); > + > +/* > + * Validate D-PHY configuration according to MIPI D-PHY specification > + * (v1.2, Section Section 6.9 "Global Operation Timing Parameters"). > + */ > +int phy_mipi_dphy_config_validate(struct phy_configure_opts_mipi_dphy *cfg) > +{ > + unsigned long ui; > + > + if (!cfg) > + return -EINVAL; Same here. > + ui = DIV_ROUND_UP(NSEC_PER_SEC, cfg->hs_clk_rate); > + > + if (cfg->clk_miss > 60) > + return -EINVAL; > + > + if (cfg->clk_post < (60 + 52 * ui)) > + return -EINVAL; > + > + if (cfg->clk_pre < 8) > + return -EINVAL; > + > + if (cfg->clk_prepare < 38 || cfg->clk_prepare > 95) > + return -EINVAL; > + > + if (cfg->clk_settle < 95 || cfg->clk_settle > 300) > + return -EINVAL; > + > + if (cfg->clk_term_en > 38) > + return -EINVAL; > + > + if (cfg->clk_trail < 60) > + return -EINVAL; > + > + if (cfg->clk_prepare + cfg->clk_zero < 300) > + return -EINVAL; > + > + if (cfg->d_term_en > 35 + 4 * ui) > + return -EINVAL; > + > + if (cfg->eot > 105 + 12 * ui) > + return -EINVAL; > + > + if (cfg->hs_exit < 100) > + return -EINVAL; > + > + if (cfg->hs_prepare < 40 + 4 * ui || > + cfg->hs_prepare > 85 + 6 * ui) > + return -EINVAL; > + > + if (cfg->hs_prepare + cfg->hs_zero < 145 + 10 * ui) > + return -EINVAL; > + > + if ((cfg->hs_settle < 85 + 6 * ui) || > + (cfg->hs_settle > 145 + 10 * ui)) > + return -EINVAL; > + > + if (cfg->hs_skip < 40 || cfg->hs_skip > 55 + 4 * ui) > + return -EINVAL; > + > + if (cfg->hs_trail < max(8 * ui, 60 + 4 * ui)) > + return -EINVAL; > + > + if (cfg->init < 100000) > + return -EINVAL; > + > + if (cfg->lpx < 50) > + return -EINVAL; > + > + if (cfg->ta_get != 5 * cfg->lpx) > + return -EINVAL; > + > + if (cfg->ta_go != 4 * cfg->lpx) > + return -EINVAL; > + > + if (cfg->ta_sure < cfg->lpx || cfg->ta_sure > 2 * cfg->lpx) > + return -EINVAL; > + > + if (cfg->wakeup < 1000000) > + return -EINVAL; > + > + return 0; > +} > +EXPORT_SYMBOL(phy_mipi_dphy_config_validate); > diff --git a/include/linux/phy/phy-mipi-dphy.h > b/include/linux/phy/phy-mipi-dphy.h index 792724145290..7656d057198f 100644 > --- a/include/linux/phy/phy-mipi-dphy.h > +++ b/include/linux/phy/phy-mipi-dphy.h > @@ -238,4 +238,10 @@ struct phy_configure_opts_mipi_dphy { > /* TODO: Add other modes (burst, commands, etc) */ > #define MIPI_DPHY_MODE_VIDEO_SYNC_PULSE BIT(0) > > +int phy_mipi_dphy_get_default_config(unsigned long pixel_clock, > + unsigned int bpp, > + unsigned int lanes, > + struct phy_configure_opts_mipi_dphy *cfg); > +int phy_mipi_dphy_config_validate(struct phy_configure_opts_mipi_dphy > *cfg); > + > #endif /* __PHY_MIPI_DPHY_H_ */ -- Regards, Laurent Pinchart