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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CEF56C6FD1F for ; Wed, 22 Mar 2023 15:09:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230160AbjCVPHT (ORCPT ); Wed, 22 Mar 2023 11:07:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59168 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230136AbjCVPHS (ORCPT ); Wed, 22 Mar 2023 11:07:18 -0400 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EEE6337725; Wed, 22 Mar 2023 08:07:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1679497636; x=1711033636; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=yZqq5Ey/D3oqpP0tAIQyzmJSJ0lVKiE9RfUsHTpt3WQ=; b=edCkaTu1jfYs0OWc6ovVxSXQTgBV9YKLCXE0BcIWFBefqZQHbubQeMhc Om11qMAtsav+tXItmrj3wFnem5wn17EDNnrSplEmWFYw2Kj9YB6ystpJ9 tqDXvTTJhGzadi02ntHd6qMYu3g8wPOTW/Gues+uLizzjiE1aBlB7k1OG LRGsbJI/JRP+bdt/HHXk9WBRY7WQ0RHKh9plZ9SdnphxTnXrPvTuTPigH 8I4Ex6Eg7Qp+l/c/3uOzKnYwDKJZwXjjAEFMnmHt9WIaJzJLrCI4IW5Cl ujupxYz1dpqZD21LXlDFXDjclInW/rF3CgeA4PvSvAJfr1RI9mnACTZ21 Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10657"; a="404121289" X-IronPort-AV: E=Sophos;i="5.98,282,1673942400"; d="scan'208";a="404121289" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Mar 2023 08:03:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10657"; a="712252989" X-IronPort-AV: E=Sophos;i="5.98,282,1673942400"; d="scan'208";a="712252989" Received: from smile.fi.intel.com ([10.237.72.54]) by orsmga008.jf.intel.com with ESMTP; 22 Mar 2023 08:03:43 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.96) (envelope-from ) id 1pf005-007AoY-2N; Wed, 22 Mar 2023 17:03:41 +0200 Date: Wed, 22 Mar 2023 17:03:41 +0200 From: Andy Shevchenko To: "Zhang, Tianfei" Cc: "richardcochran@gmail.com" , "netdev@vger.kernel.org" , "linux-fpga@vger.kernel.org" , "ilpo.jarvinen@linux.intel.com" , "Gomes, Vinicius" , "pierre-louis.bossart@linux.intel.com" , "Pagani, Marco" , "Weight, Russell H" , "matthew.gerlach@linux.intel.com" , "nico@fluxnic.net" , "Khadatare, RaghavendraX Anand" Subject: Re: [PATCH v2] ptp: add ToD device driver for Intel FPGA cards Message-ID: References: <20230322143547.233250-1-tianfei.zhang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo Precedence: bulk List-ID: X-Mailing-List: linux-fpga@vger.kernel.org On Wed, Mar 22, 2023 at 02:59:21PM +0000, Zhang, Tianfei wrote: > > -----Original Message----- > > From: Andy Shevchenko > > Sent: Wednesday, March 22, 2023 10:49 PM > > To: Zhang, Tianfei > > Cc: richardcochran@gmail.com; netdev@vger.kernel.org; linux- > > fpga@vger.kernel.org; ilpo.jarvinen@linux.intel.com; Gomes, Vinicius > > ; pierre-louis.bossart@linux.intel.com; Pagani, Marco > > ; Weight, Russell H ; > > matthew.gerlach@linux.intel.com; nico@fluxnic.net; Khadatare, RaghavendraX > > Anand > > Subject: Re: [PATCH v2] ptp: add ToD device driver for Intel FPGA cards > > > > On Wed, Mar 22, 2023 at 10:35:47AM -0400, Tianfei Zhang wrote: > > > Adding a DFL (Device Feature List) device driver of ToD device for > > > Intel FPGA cards. > > > > > > The Intel FPGA Time of Day(ToD) IP within the FPGA DFL bus is exposed > > > as PTP Hardware clock(PHC) device to the Linux PTP stack to > > > synchronize the system clock to its ToD information using phc2sys > > > utility of the Linux PTP stack. The DFL is a hardware List within > > > FPGA, which defines a linked list of feature headers within the device > > > MMIO space to provide an extensible way of adding subdevice features. ... > > > + dt->ptp_clock = ptp_clock_register(&dt->ptp_clock_ops, dev); > > > + if (IS_ERR_OR_NULL(dt->ptp_clock)) > > > + return dev_err_probe(dt->dev, PTR_ERR_OR_ZERO(dt->ptp_clock), > > > + "Unable to register PTP clock\n"); > > > + > > > + return 0; > > > > Can be as simple as: > > > > ret = PTR_ERR_OR_ZERO(dt->ptp_clock); > > return dev_err_probe(dt->dev, ret, "Unable to register PTP clock\n"); > > This should be : > ret = PTR_ERR_OR_ZERO(dt->ptp_clock); > if (ret) > return dev_err_probe(dt->dev, ret, "Unable to register PTP clock\n"); > return 0; It depends how you treat the NULL from ptp_clock_register() and why driver will be still bound to the device even if it doesn't provide PTP facility. Either way you need to thing about this error handling and come up with something that you can explain why it's done this way and not another. > But this will be introduced one more local variable "ret" in this function. Is it a problem? -- With Best Regards, Andy Shevchenko