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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 045CFC433F5 for ; Thu, 29 Sep 2022 11:34:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=zqKvL936jLXj4AngIStDO8QattvsUjHDLmGizqKiwPI=; b=W3XwwoOisDjvGp b4QSQHQWGJ5AkNrQ3TuZJTAnYYCXGcIJdkwn7Il7KDmoB+vQVS9I1qw9SPcqwFtcrU+QFM4EW6+Ai E22tq5JCntxDa/IZBbw2m3x4a7sd/vFQ07oqeWGFQ7ZHdS6XqxmfrCTGk4v1HQEftddnzFbyD8mPY 0itaEwn8DtI85hw+a1frV3Ilvngkw/jWWtRTF28RmURJGBZ+kRl6EYOGs9A50rO7oQQZ5R9OJJ+mD yiYniAEISqXQVi86prb0W9PxACjkLdB+e49vc0rK0I//OmVk1Tty00DoxSvgcJ9cXMuPyv13LvPpE 6HI+4O/+xof9AOIp+M2Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1odrnf-002fiA-53; Thu, 29 Sep 2022 11:33:55 +0000 Received: from mail-ej1-x634.google.com ([2a00:1450:4864:20::634]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1odrna-002ffR-4a for linux-arm-kernel@lists.infradead.org; Thu, 29 Sep 2022 11:33:53 +0000 Received: by mail-ej1-x634.google.com with SMTP id a26so2168664ejc.4 for ; Thu, 29 Sep 2022 04:33:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=resnulli-us.20210112.gappssmtp.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date; bh=yECNLisTYoTEe2lUuczdT4RhsomyATai+SGMeUtC7d4=; b=3IMEPtGbYyfvXrWiKrAe8PT0Pa2tCHO9tDDI5zUqL2DzaYJ3NBP/AnfhRHHgifz8M8 ZsvpEcnJP7uQBNVMO0Z+P//D/5IVKulgoDb8x2v1W6JGbJ5B95LKdGv+RFAlcRGwsitZ yFtp2s6P9ufhYOPaqUJQEAyeqQQzcnKzMrxsL8CcyoS/ZDElj8cSb6CvmI/SZCYu87hW zLzuxctheLJtzQo/zCPqyy8u9xvPGz5DwAg/Qy+vk7jRaB2mPwS9BmyRB4F0+qqITm+l U40lJfkHXEn7C+ND+nO8OU6BZOQOjTGuJJEx62zbiT5oCLXA9eFfupfftR9KyoiMbbB/ MumQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date; bh=yECNLisTYoTEe2lUuczdT4RhsomyATai+SGMeUtC7d4=; b=dEl22CiULbcpgHD0pMQe8tVE8e6+08xlUUWOZ1mtyfzzv/Pyk7AxR+KVOPqa+XrE/5 7Z1d8Rf83iWC8ikGzBBbXXLdmDp1CCMJm16RUx9yOErZkBN29JZKrxozoMXfx+3yx5Iz BzABNXpfSRFOV73ARHRRs9cvbx7kUODJ2Q09wuQefFh4Hwf9UJ4aio0MRpoKCF4KLlI/ /rSoNXYlFNf8SS9WFFVVjoYVU1aMUxTlOtlZuZFyO570hbp4/MDF5LG2GCa9mo4+FQGX CjDnD48mtdvbYdz+uCIBMy2CJ3Z/2KFAmL4eUkYexXa+ayexQmk12pld0NSqslKBFEr0 ZWkQ== X-Gm-Message-State: ACrzQf237fJcEbCwnV1RdFbeH50tUF1SQqksmCgOEDOHBxeja9cxP6l6 VNOxn5Z+Nct6cZiZxO/9JBVVt1f7KR/3ZlDP X-Google-Smtp-Source: AMsMyM5hkisoXcutfUne1QndObmMEelNqGCE70ythxufmVYTFu5+ADdwl/GBD+QFxUvKz2+/VhmOgw== X-Received: by 2002:a17:907:a067:b0:77b:9672:3f83 with SMTP id ia7-20020a170907a06700b0077b96723f83mr2317022ejc.523.1664451226665; Thu, 29 Sep 2022 04:33:46 -0700 (PDT) Received: from localhost (host-213-179-129-39.customer.m-online.net. [213.179.129.39]) by smtp.gmail.com with ESMTPSA id c7-20020a170906528700b00781f1e57ebbsm3813370ejm.49.2022.09.29.04.33.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Sep 2022 04:33:45 -0700 (PDT) Date: Thu, 29 Sep 2022 13:33:44 +0200 From: Jiri Pirko To: Vadim Fedorenko Cc: Jakub Kicinski , Arkadiusz Kubalewski , Jonathan Lemon , Vadim Fedorenko , Aya Levin , netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org Subject: Re: [RFC PATCH v2 3/3] ptp_ocp: implement DPLL ops Message-ID: References: <20220626192444.29321-1-vfedorenko@novek.ru> <20220626192444.29321-4-vfedorenko@novek.ru> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220626192444.29321-4-vfedorenko@novek.ru> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220929_043350_434910_DC389DF0 X-CRM114-Status: GOOD ( 21.18 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Sun, Jun 26, 2022 at 09:24:44PM CEST, vfedorenko@novek.ru wrote: >From: Vadim Fedorenko > >Implement DPLL operations in ptp_ocp driver. > >Signed-off-by: Vadim Fedorenko >--- > drivers/ptp/Kconfig | 1 + > drivers/ptp/ptp_ocp.c | 169 ++++++++++++++++++++++++++++++-------- > include/uapi/linux/dpll.h | 2 + > 3 files changed, 136 insertions(+), 36 deletions(-) > >diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig >index 458218f88c5e..f74846ebc177 100644 >--- a/drivers/ptp/Kconfig >+++ b/drivers/ptp/Kconfig >@@ -176,6 +176,7 @@ config PTP_1588_CLOCK_OCP > depends on !S390 > depends on COMMON_CLK > select NET_DEVLINK >+ select DPLL > help > This driver adds support for an OpenCompute time card. > >diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c >index e59ea2173aac..f830625a63a3 100644 >--- a/drivers/ptp/ptp_ocp.c >+++ b/drivers/ptp/ptp_ocp.c >@@ -21,6 +21,7 @@ > #include > #include > #include >+#include > > #define PCI_VENDOR_ID_FACEBOOK 0x1d9b > #define PCI_DEVICE_ID_FACEBOOK_TIMECARD 0x0400 >@@ -336,6 +337,7 @@ struct ptp_ocp { > struct ptp_ocp_signal signal[4]; > struct ptp_ocp_sma_connector sma[4]; > const struct ocp_sma_op *sma_op; >+ struct dpll_device *dpll; > }; > > #define OCP_REQ_TIMESTAMP BIT(0) >@@ -660,18 +662,19 @@ static DEFINE_IDR(ptp_ocp_idr); > struct ocp_selector { > const char *name; > int value; >+ int dpll_type; > }; > > static const struct ocp_selector ptp_ocp_clock[] = { >- { .name = "NONE", .value = 0 }, >- { .name = "TOD", .value = 1 }, >- { .name = "IRIG", .value = 2 }, >- { .name = "PPS", .value = 3 }, >- { .name = "PTP", .value = 4 }, >- { .name = "RTC", .value = 5 }, >- { .name = "DCF", .value = 6 }, >- { .name = "REGS", .value = 0xfe }, >- { .name = "EXT", .value = 0xff }, >+ { .name = "NONE", .value = 0, .dpll_type = 0 }, >+ { .name = "TOD", .value = 1, .dpll_type = 0 }, >+ { .name = "IRIG", .value = 2, .dpll_type = 0 }, >+ { .name = "PPS", .value = 3, .dpll_type = 0 }, >+ { .name = "PTP", .value = 4, .dpll_type = 0 }, >+ { .name = "RTC", .value = 5, .dpll_type = 0 }, >+ { .name = "DCF", .value = 6, .dpll_type = 0 }, >+ { .name = "REGS", .value = 0xfe, .dpll_type = 0 }, >+ { .name = "EXT", .value = 0xff, .dpll_type = 0 }, > { } > }; > >@@ -680,37 +683,37 @@ static const struct ocp_selector ptp_ocp_clock[] = { > #define SMA_SELECT_MASK GENMASK(14, 0) > > static const struct ocp_selector ptp_ocp_sma_in[] = { >- { .name = "10Mhz", .value = 0x0000 }, >- { .name = "PPS1", .value = 0x0001 }, >- { .name = "PPS2", .value = 0x0002 }, >- { .name = "TS1", .value = 0x0004 }, >- { .name = "TS2", .value = 0x0008 }, >- { .name = "IRIG", .value = 0x0010 }, >- { .name = "DCF", .value = 0x0020 }, >- { .name = "TS3", .value = 0x0040 }, >- { .name = "TS4", .value = 0x0080 }, >- { .name = "FREQ1", .value = 0x0100 }, >- { .name = "FREQ2", .value = 0x0200 }, >- { .name = "FREQ3", .value = 0x0400 }, >- { .name = "FREQ4", .value = 0x0800 }, >- { .name = "None", .value = SMA_DISABLE }, >+ { .name = "10Mhz", .value = 0x0000, .dpll_type = DPLL_TYPE_EXT_10MHZ }, >+ { .name = "PPS1", .value = 0x0001, .dpll_type = DPLL_TYPE_EXT_1PPS }, >+ { .name = "PPS2", .value = 0x0002, .dpll_type = DPLL_TYPE_EXT_1PPS }, >+ { .name = "TS1", .value = 0x0004, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "TS2", .value = 0x0008, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "IRIG", .value = 0x0010, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "DCF", .value = 0x0020, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "TS3", .value = 0x0040, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "TS4", .value = 0x0080, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "FREQ1", .value = 0x0100, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "FREQ2", .value = 0x0200, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "FREQ3", .value = 0x0400, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "FREQ4", .value = 0x0800, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "None", .value = SMA_DISABLE, .dpll_type = DPLL_TYPE_NONE }, > { } > }; > > static const struct ocp_selector ptp_ocp_sma_out[] = { >- { .name = "10Mhz", .value = 0x0000 }, >- { .name = "PHC", .value = 0x0001 }, >- { .name = "MAC", .value = 0x0002 }, >- { .name = "GNSS1", .value = 0x0004 }, >- { .name = "GNSS2", .value = 0x0008 }, >- { .name = "IRIG", .value = 0x0010 }, >- { .name = "DCF", .value = 0x0020 }, >- { .name = "GEN1", .value = 0x0040 }, >- { .name = "GEN2", .value = 0x0080 }, >- { .name = "GEN3", .value = 0x0100 }, >- { .name = "GEN4", .value = 0x0200 }, >- { .name = "GND", .value = 0x2000 }, >- { .name = "VCC", .value = 0x4000 }, >+ { .name = "10Mhz", .value = 0x0000, .dpll_type = DPLL_TYPE_EXT_10MHZ }, >+ { .name = "PHC", .value = 0x0001, .dpll_type = DPLL_TYPE_INT_OSCILLATOR }, >+ { .name = "MAC", .value = 0x0002, .dpll_type = DPLL_TYPE_INT_OSCILLATOR }, >+ { .name = "GNSS1", .value = 0x0004, .dpll_type = DPLL_TYPE_GNSS }, >+ { .name = "GNSS2", .value = 0x0008, .dpll_type = DPLL_TYPE_GNSS }, >+ { .name = "IRIG", .value = 0x0010, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "DCF", .value = 0x0020, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "GEN1", .value = 0x0040, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "GEN2", .value = 0x0080, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "GEN3", .value = 0x0100, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "GEN4", .value = 0x0200, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "GND", .value = 0x2000, .dpll_type = DPLL_TYPE_CUSTOM }, >+ { .name = "VCC", .value = 0x4000, .dpll_type = DPLL_TYPE_CUSTOM }, > { } > }; > >@@ -3713,6 +3716,90 @@ ptp_ocp_detach(struct ptp_ocp *bp) > device_unregister(&bp->dev); > } > >+static int ptp_ocp_dpll_get_status(struct dpll_device *dpll) >+{ >+ struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); Unnecessary cast. >+ int sync; >+ >+ sync = ioread32(&bp->reg->status) & OCP_STATUS_IN_SYNC; Just return without having "sync" variable. >+ return sync; >+} >+ >+static int ptp_ocp_dpll_get_lock_status(struct dpll_device *dpll) >+{ >+ struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); >+ int sync; >+ >+ sync = ioread32(&bp->reg->status) & OCP_STATUS_IN_SYNC; >+ return sync; This is very odd. You return something you read from device directly over what likes to be an abstract API. Is it supposed to be a bool? If yes, make it so. >+} >+ >+static int ptp_ocp_sma_get_dpll_type(struct ptp_ocp *bp, int sma_nr) Type should be most probably enum instead of int. >+{ >+ struct ocp_selector *tbl; >+ u32 val; >+ >+ if (bp->sma[sma_nr].mode == SMA_MODE_IN) >+ tbl = bp->sma_op->tbl[0]; >+ else >+ tbl = bp->sma_op->tbl[1]; >+ >+ val = ptp_ocp_sma_get(bp, sma_nr); >+ return tbl[val].dpll_type; >+} >+ >+static int ptp_ocp_dpll_type_supported(struct dpll_device *dpll, int sma, int type, int dir) >+{ >+ struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); >+ struct ocp_selector *tbl = bp->sma_op->tbl[dir]; >+ int i; >+ >+ for (i = 0; i < sizeof(*tbl); i++) { >+ if (tbl[i].dpll_type == type) >+ return 1; >+ } >+ return 0; 1/0? Bool please. >+} >+ >+static int ptp_ocp_dpll_get_source_type(struct dpll_device *dpll, int sma) >+{ >+ struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); >+ >+ if (bp->sma[sma].mode != SMA_MODE_IN) >+ return -1; >+ >+ return ptp_ocp_sma_get_dpll_type(bp, sma); enum. >+} >+ >+static int ptp_ocp_dpll_get_source_supported(struct dpll_device *dpll, int sma, int type) >+{ >+ return ptp_ocp_dpll_type_supported(dpll, sma, type, 0); >+} >+ >+static int ptp_ocp_dpll_get_output_type(struct dpll_device *dpll, int sma) >+{ >+ struct ptp_ocp *bp = (struct ptp_ocp *)dpll_priv(dpll); >+ >+ if (bp->sma[sma].mode != SMA_MODE_OUT) >+ return -1; >+ >+ return ptp_ocp_sma_get_dpll_type(bp, sma); >+} >+ >+static int ptp_ocp_dpll_get_output_supported(struct dpll_device *dpll, int sma, int type) >+{ >+ return ptp_ocp_dpll_type_supported(dpll, sma, type, 1); 1? Have the "dir" to be enum please. >+} >+ >+static struct dpll_device_ops dpll_ops = { >+ .get_status = ptp_ocp_dpll_get_status, >+ .get_lock_status = ptp_ocp_dpll_get_lock_status, >+ .get_source_type = ptp_ocp_dpll_get_source_type, >+ .get_source_supported = ptp_ocp_dpll_get_source_supported, >+ .get_output_type = ptp_ocp_dpll_get_output_type, >+ .get_output_supported = ptp_ocp_dpll_get_output_supported, >+}; >+ > static int > ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) > { >@@ -3768,6 +3855,14 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > ptp_ocp_info(bp); > devlink_register(devlink); >+ >+ bp->dpll = dpll_device_alloc(&dpll_ops, ARRAY_SIZE(bp->sma), ARRAY_SIZE(bp->sma), bp); >+ if (!bp->dpll) { >+ dev_err(&pdev->dev, "dpll_device_alloc failed\n"); >+ return 0; >+ } >+ dpll_device_register(bp->dpll); I wonder, why you don't have alloc-register and unregister-free squashed? >+ > return 0; > > out: >@@ -3785,6 +3880,8 @@ ptp_ocp_remove(struct pci_dev *pdev) > struct ptp_ocp *bp = pci_get_drvdata(pdev); > struct devlink *devlink = priv_to_devlink(bp); > >+ dpll_device_unregister(bp->dpll); >+ dpll_device_free(bp->dpll); > devlink_unregister(devlink); > ptp_ocp_detach(bp); > pci_disable_device(pdev); >diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h >index 7ce45c6b4fd4..5e8c46712d18 100644 >--- a/include/uapi/linux/dpll.h >+++ b/include/uapi/linux/dpll.h >@@ -41,11 +41,13 @@ enum dpll_genl_attr { > > /* DPLL signal types used as source or as output */ > enum dpll_genl_signal_type { >+ DPLL_TYPE_NONE, > DPLL_TYPE_EXT_1PPS, > DPLL_TYPE_EXT_10MHZ, > DPLL_TYPE_SYNCE_ETH_PORT, > DPLL_TYPE_INT_OSCILLATOR, > DPLL_TYPE_GNSS, >+ DPLL_TYPE_CUSTOM, This hunk does not belong to this patch. > > __DPLL_TYPE_MAX, > }; >-- >2.27.0 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel