Received: by 2002:ad5:474a:0:0:0:0:0 with SMTP id i10csp6443526imu; Wed, 30 Jan 2019 15:02:12 -0800 (PST) X-Google-Smtp-Source: ALg8bN6TGsg0iPABWelPmI6N3bdQRcxhaPAEp5e7A8xmyvmHn41Nq5Z0Co9lp+fC870YDGDdOc8f X-Received: by 2002:a62:11c7:: with SMTP id 68mr32290235pfr.21.1548889332243; Wed, 30 Jan 2019 15:02:12 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1548889332; cv=none; d=google.com; s=arc-20160816; b=OLw+eVZxp7HLUd9LZDpZNltVoz5WXi3Hc0eZmWLPuZCtSy2Dzv64yrmr5HRA03FWRB s7eVDilTatZPlBel23B0erUM2+VTtT4aika/GHe6XuSKooSxZ4jXKalIJs11AhZJA6ld DWUKnbeDnTSsWVSECROvhiOHjP5E9TBnVu+h9wbo9AOJOy44vQA6XJiwbSiTCNvPYAfx dzizqVAYdiVgXYhG/cZ/psZeZOwl0ueqUrR8AHl5MYCnX/Cvz/8EMuHZxgpzLycjli3/ jPLjpjFvwoYmNkjkWxBA9V9ESul1wqghr+Yuz9mE9nU39QZaHUKfRXam5/RdCLEWkBD8 2rcA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=hwsXWhqhObmoCvm6HCrYpY1W9DGqdyw1V/0UwP7b1B0=; b=QvtB5JuquvoYUbuEMfucQuv80WnSP90G/u3AV791FVi10wzqqnnj3XKdWPc2vNFK1g Q8MLnjN2ZVGciefdwm1tObzGNDa5eoPkv6GrZQWZkjbQQTzAF68fMXd8z6wcwNzVp2jM 8EQe/ft+uC/eWULX1PrkBcs4LpG9VwpdRN3LcCoIOvS625EpmDjcl3c55NMqJkeiz1Yz BHuzGedn3wD0GyQL7pnB2G0xsA4EVrlOXegQgWeVRzheBW9Txx9Yyz2zhFWAygXRoxrh I3lkuTYVuxZGYLpmL0KzZt84oOO260tIK7yFdnPLSp4aUdR5I/y0h3wZcsDXZ+pGyhP4 TQBg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=ZNfK2ulf; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id k18si2684066pfd.241.2019.01.30.15.01.57; Wed, 30 Jan 2019 15:02:12 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=ZNfK2ulf; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732958AbfA3VRX (ORCPT + 99 others); Wed, 30 Jan 2019 16:17:23 -0500 Received: from mail-pl1-f195.google.com ([209.85.214.195]:44167 "EHLO mail-pl1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729410AbfA3VRW (ORCPT ); Wed, 30 Jan 2019 16:17:22 -0500 Received: by mail-pl1-f195.google.com with SMTP id e11so388075plt.11; Wed, 30 Jan 2019 13:17:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hwsXWhqhObmoCvm6HCrYpY1W9DGqdyw1V/0UwP7b1B0=; b=ZNfK2ulfgT4A1e4nhgSIqo9tRFGidREQLZip6W/re+jWfr6HzH/qJ5Y5gFOVSHDJ5H rPhIA7GM9rAoU61ouPoWRGn2QQLABkeefcwMZwpECS/Fpno1z0uVmxlfuTW/3oaEFD2z fNFP2s9XXll2laQ2J4CgNqhRSzBwZo1CQTePowSo0B5O3e05dLL22EzdbyCXZyFeRR9e UxMz7guVQqgIEfjjO9fzOghw+ZrsYJPkhGjrTyKrZZRifX90J+T5TON+24tUU460nZV/ k0kyZIxHaOohNKoR3YaurWkSCAK0hmdZX7+a+ErbJ8aFGYzdT2NRKG9SuKmBqHU75jxM t4Gg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=hwsXWhqhObmoCvm6HCrYpY1W9DGqdyw1V/0UwP7b1B0=; b=bA3JCL4mcaxPcaOxgFEPjaT2TTw3x2hZhGapdxodpUEGuiMp3MqJtSm+4gSf6I0okL CqW2QIrwduhkIa11TAFrBpmsUwZ5ef2AjCdwmJWXPSZApOedsKwTX6585dgJKqB1xmqD px2aFuBblCM9vOkYBXi7KUbGWVs35TivmiTi1isMitYdB4mtibYV++ngxzgFypSYM2Vq hfXcVX0xJGVybCNp+//R1TKDvBBz/OteZO0gDIGW0jt37/n9OrARC5audC5NAAoZ6awu /afINpUq0AF/NRCUN+rTYcWiFYZ556rcXlZybn05qjYMbGFaW58G4OFdb2nJuWSh6/7G 1HWQ== X-Gm-Message-State: AJcUukdc7PxtVv6+KoeRfXgsqtp0uu9VhxQ9VQMdkhDgTWYNwov8zFil mowCsBeppzTRM2Ca2TJR7Z5HVHovzghZ5FI8MPGawSct X-Received: by 2002:a17:902:2468:: with SMTP id m37mr31965097plg.314.1548883041547; Wed, 30 Jan 2019 13:17:21 -0800 (PST) MIME-Version: 1.0 References: <0b74e9ad12360b56bc0a3c2ca972798c424f2610.1548790896.git.lsun@mellanox.com> In-Reply-To: From: Andy Shevchenko Date: Wed, 30 Jan 2019 23:17:09 +0200 Message-ID: Subject: Re: [PATCH v1 1/1] platform/mellanox: Add bootctl driver for Mellanox BlueField Soc To: Liming Sun Cc: Andy Shevchenko , Darren Hart , Vadim Pasternak , David Woods , Platform Driver , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jan 30, 2019 at 10:47 PM Liming Sun wrote: > > First of all, is it a real watchdog with a driver? I think watchdog in > > that case should be set up through standard watchdog facilities. > > This is not a watchdog driver itself. Instead, it provides interface to > user-space and use ARM SMC calls to ATF to configure registers and > watchdog. I'll update the commit message in v2 to clarify it. Hmm... For example Intel MID platforms have SCU (system controller unit) that provides a watchdog facility. In the kernel we have a watchdog driver for that. Can't you do similar for your case? > > > + if (mlxbf_bootctl_smc_call1(MLXBF_BOOTCTL_SET_POST_RESET_WDOG, > > > + watchdog) < 0) > > > + return -EINVAL; > > > > If that call returns an error it shouldn't be shadowed here. > > Not sure I understand this comment correctly or not. > This function is defined by DRIVER_ATTR_RW(), which appears to expect > ssize_t or an error code as return value according to other examples I saw. What is returned by mlx...call1() should be propagated to the actual caller. Same comment for all similar cases. > > > + lc_state &= (MLXBF_BOOTCTL_SB_MODE_TEST_MASK | > > > + MLXBF_BOOTCTL_SB_MODE_SECURE_MASK); > > > > Better to split like > > > > xxx = > > (A | B); > > It seems hard to do "(A | B);" within 80 characters plus the indents. Repeating myself, it's still better than your variant for readability. > > > + if (res.a0 != 0x89c036b4 || res.a1 != 0x11e6e7d7 || > > > + res.a2 != 0x1a009787 || res.a3 != 0xc4bf00ca) > > > > What is this?! > > > > Can you use UUID API? > > Yes, it is UUID comparison. The SMC call returns four 'unsigned long' from ATF > to represent the UUID. There seems no existing APIs in uapi/linux/uuid.h to > compare such special format. How about replacing it with comment and MACROs > instead of the hardcoded values to make it more readable? Should be no magic numbers involved inside the function at the end. Use descriptive definitions and I still recommend to give a look at UUID API how it can be utilized here. (hint: Thunderbolt hw is operating with integers, though driver uses UUID API at the end) -- With Best Regards, Andy Shevchenko