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 smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (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 CABFCE7542B for ; Tue, 3 Oct 2023 06:27:16 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 4D32160B7C; Tue, 3 Oct 2023 06:27:16 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 4D32160B7C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=osuosl.org; s=default; t=1696314436; bh=6F1MKVhIsFTXdtfET8jkuHrBctcQzJbRB642wp7LHOM=; h=Date:From:To:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: Cc:From; b=JLdzB9uhZvAHlTm7tOLbhS5RJ9/MOtfqlnS9b7gSCXReQl+AnYc74KEk2ZLhvONd0 HxNI6QJ51OiH7g1mIw+5zeP3JMefRnfZfqjo2cJ23rjfncfaOl+wd3DufO6cRFh1ia FPlZYPpga2peZ1QprgOZSbQ89U5V1KZSavV/ikI02VtVXBOIOV1LdqA5Q48aC/QjkY iIdOPlGVCEGvWjB9TBczGpm+n5FefCuFflm//Ark+tMuuqfpzstqm5qoxHNIBC98UL XB3Tb1kUeb3VZDThqOBunTph/1SHQVzruWzGCvnM2XfsIa4p9z3ZWuOl1WwVeFh/74 Zku0nIbClzKhw== X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id OrVjLm9CaPyv; Tue, 3 Oct 2023 06:27:15 +0000 (UTC) Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by smtp3.osuosl.org (Postfix) with ESMTP id 3D9CE60C11; Tue, 3 Oct 2023 06:27:15 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 3D9CE60C11 Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by ash.osuosl.org (Postfix) with ESMTP id DAB131BF393 for ; Tue, 3 Oct 2023 06:27:12 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id B270740140 for ; Tue, 3 Oct 2023 06:27:12 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org B270740140 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Y7n7IlnTj82e for ; Tue, 3 Oct 2023 06:27:10 +0000 (UTC) Received: from mail-ed1-x532.google.com (mail-ed1-x532.google.com [IPv6:2a00:1450:4864:20::532]) by smtp2.osuosl.org (Postfix) with ESMTPS id 5ED6E4004A for ; Tue, 3 Oct 2023 06:27:09 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 5ED6E4004A Received: by mail-ed1-x532.google.com with SMTP id 4fb4d7f45d1cf-5347e657a11so764930a12.2 for ; Mon, 02 Oct 2023 23:27:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696314428; x=1696919228; 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 :message-id:reply-to; bh=nasBPIo/xHcUBNpodGyPU6trXTolky3DJWsttEP7pMQ=; b=opPrEHkMqMtOQmrMzwXqWTQmg/zfZqx9z69cpZtXNUiO1eIpoSFWIlUXCxM31yzfdf WsToIpFrdPMN4KwMNPE/yGDpzE6XA8z7r7XZw0YlaYPcdWlATu1Riynac0i283kzCTKf k/Y6lbTB/GtFXylDKIGA11x4mhmVsyGJiEsW1X35/3gtfxeuVbrqo400YZBXM/s56SQa wZjemRIbQMFKwO2ZRnepJliU9z3keKbLsUjwv6/OCqEoIv1GmFH+MLOCcId1G0aIkcqJ krjUmxTuESTD/XyoWsQa0hhVs7aBkApYP+HpB+bRDFQJLw7LlZU5m2rquweZPO3PfsG/ bZLg== X-Gm-Message-State: AOJu0Yw31xCXRAJ27S6z2SJOC9keUPP1b9NcQXxRU4y25nILy4pU5EIr wyOTjBzpRqyC7jfPmaI2qc15hg== X-Google-Smtp-Source: AGHT+IGktzEJPQwQ4FrIOHaSByngUhSzABrutCkp7xAGo5V5R4BfWDuJKH4B8vDl7eGG7nhBjMIC2g== X-Received: by 2002:a50:ec89:0:b0:533:3d2b:7473 with SMTP id e9-20020a50ec89000000b005333d2b7473mr11167966edr.12.1696314427737; Mon, 02 Oct 2023 23:27:07 -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 b20-20020aa7c914000000b0053331f9094dsm323417edt.52.2023.10.02.23.27.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 02 Oct 2023 23:27:07 -0700 (PDT) Date: Tue, 3 Oct 2023 08:27:06 +0200 From: Jiri Pirko To: "Kubalewski, Arkadiusz" Message-ID: References: <20230927092435.1565336-1-arkadiusz.kubalewski@intel.com> <20230927092435.1565336-4-arkadiusz.kubalewski@intel.com> <4018c0b0-b288-ff60-09be-7ded382f4a82@linux.dev> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=resnulli-us.20230601.gappssmtp.com; s=20230601; t=1696314428; x=1696919228; darn=lists.osuosl.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=nasBPIo/xHcUBNpodGyPU6trXTolky3DJWsttEP7pMQ=; b=PTQKYNiEdmV5bfJbKcDA7RAcaUukGVW/9FLLCMPbnJXNECm1VndJrcq1fN4GLUoe1h lShOQohfOZ2DVeHcDeMulQIbp28A/HsOm5MAD/EVQ9JImiGDy3CC94Csv4ggfH4mpGJP DGQqvyUtQooEu9c+OqGqVk6mRiYWYtaTQnng7jEIsMSCfCnM/17H2zEmeXRmmA2qSquv hOb8ZHdcbnmkjJ0bdscw0GJeIcUW1WAf8PvLEeNcYg7jE5gu7D2s16X94KW1hSWMSSGJ fObwIhQKtmmc5AYbzBPpSGmkaq/37L83IrwwBS51l2+vu07+z+SlGUI89O2P6nID3DGG 3DSQ== X-Mailman-Original-Authentication-Results: smtp2.osuosl.org; dkim=pass (2048-bit key) header.d=resnulli-us.20230601.gappssmtp.com header.i=@resnulli-us.20230601.gappssmtp.com header.a=rsa-sha256 header.s=20230601 header.b=PTQKYNiE Subject: Re: [Intel-wired-lan] [PATCH net-next 3/4] dpll: netlink/core: add support for pin-dpll signal phase offset/adjust X-BeenThere: intel-wired-lan@osuosl.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Wired Ethernet Linux Kernel Driver Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Vadim Fedorenko , "linux-doc@vger.kernel.org" , "netdev@vger.kernel.org" , "corbet@lwn.net" , "Brandeburg, Jesse" , "Nguyen, Anthony L" , "kuba@kernel.org" , "intel-wired-lan@lists.osuosl.org" , "pabeni@redhat.com" , "davem@davemloft.net" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: intel-wired-lan-bounces@osuosl.org Sender: "Intel-wired-lan" Tue, Oct 03, 2023 at 01:10:39AM CEST, arkadiusz.kubalewski@intel.com wrote: >>From: Intel-wired-lan On Behalf Of >>Vadim Fedorenko >>Sent: Monday, October 2, 2023 5:09 PM >> >>On 02/10/2023 16:04, Jiri Pirko wrote: >>> Mon, Oct 02, 2023 at 04:32:30PM CEST, arkadiusz.kubalewski@intel.com >>> wrote: >>>>> From: Vadim Fedorenko >>>>> Sent: Wednesday, September 27, 2023 8:09 PM >>>>> >>>>> On 27/09/2023 10:24, Arkadiusz Kubalewski wrote: >>>>>> Add callback op (get) for pin-dpll phase-offset measurment. >>>>>> Add callback ops (get/set) for pin signal phase adjustment. >>>>>> Add min and max phase adjustment values to pin proprties. >>>>>> Invoke get callbacks when filling up the pin details to provide user >>>>>> with phase related attribute values. >>>>>> Invoke phase-adjust set callback when phase-adjust value is provided >>>>>> for >>>>>> pin-set request. >>>>>> >>>>>> Signed-off-by: Arkadiusz Kubalewski >>>>> >>>>> [...] >>>>> >>>>>> +static int >>>>>> +dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr >>>>>> *phase_adj_attr, >>>>>> + struct netlink_ext_ack *extack) >>>>>> +{ >>>>>> + struct dpll_pin_ref *ref; >>>>>> + unsigned long i; >>>>>> + s32 phase_adj; >>>>>> + int ret; >>>>>> + >>>>>> + phase_adj = nla_get_s32(phase_adj_attr); >>>>>> + if (phase_adj > pin->prop->phase_range.max || >>>>>> + phase_adj < pin->prop->phase_range.min) { >>>>>> + NL_SET_ERR_MSG(extack, "phase adjust value not supported"); >>>>>> + return -EINVAL; >>>>>> + } >>>>>> + xa_for_each(&pin->dpll_refs, i, ref) { >>>>>> + const struct dpll_pin_ops *ops = dpll_pin_ops(ref); >>>>>> + struct dpll_device *dpll = ref->dpll; >>>>>> + >>>>>> + if (!ops->phase_adjust_set) >>>>>> + return -EOPNOTSUPP; >>>>> >>>>> I'm thinking about this part. We can potentially have dpll devices with >>>>> different expectations on phase adjustments, right? And if one of them >>>>> won't be able to adjust phase (or will fail in the next line), then >>>>> netlink will return EOPNOTSUPP while _some_ of the devices will be >>>>> adjusted. Doesn't look great. Can we think about different way to apply >>>>> the change? >>>>> >>>> >>>> Well makes sense to me. >>>> >>>> Does following makes sense as a fix? >>>> We would call op for all devices which has been provided with the op. >>>> If device has no op -> add extack error, continue >>> >>> Is it real to expect some of the device support this and others don't? >>> Is it true for ice? >>> If not, I would got for all-or-nothing here. >>> >> >>But nothing blocks vendors to provide such configuration. Should we >>rollback the configuration? Otherwise we can easily make it >>inconsistent. > >Good point, in such case rollback might be required. > >> >>I'm more thinking of checking if all the devices returned error (or >>absence of operation callback) and then return error instead of 0 with >>extack filled in. >> > >Well, what if different devices would return different errors? >In general we would have to keep track of the error values returned in >such case.. Assuming one is different than the other - still need to error >extack them out? I guess it would be easier to return common error if there In this case, it is common to return the first error hit and bail out, not trying the rest. >were only failures and let the driver fill the errors on extack, smt like: > > int miss_cb_num = 0, dev_num = 0, err_num; > > xa_for_each(&pin->dpll_refs, i, ref) { > const struct dpll_pin_ops *ops = dpll_pin_ops(ref); > struct dpll_device *dpll = ref->dpll; > > dev_num++; > if (!ops->phase_adjust_set) { > miss_cb_num++; > continue; > } > ret = ops->phase_adjust_set(pin, > dpll_pin_on_dpll_priv(dpll, pin), > dpll, dpll_priv(dpll), phase_adj, > extack); > if (ret) > err_num++; > } > if (dev_num == miss_cb_num) > return -EOPNOTSUPP; > if (dev_num == err_num) > return -EINVAL; > __dpll_pin_change_ntf(pin); > return 0; > >?? > >Thank you! >Arkadiusz > >>> >>>> If device fails to set -> add extack error, continue >>>> Function always returns 0. >>>> >>>> Thank you! >>>> Arkadiusz >>>> >>>>> >>>>>> + ret = ops->phase_adjust_set(pin, >>>>>> + dpll_pin_on_dpll_priv(dpll, pin), >>>>>> + dpll, dpll_priv(dpll), phase_adj, >>>>>> + extack); >>>>>> + if (ret) >>>>>> + return ret; >>>>>> + } >>>>>> + __dpll_pin_change_ntf(pin); >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >> >>_______________________________________________ >>Intel-wired-lan mailing list >>Intel-wired-lan@osuosl.org >>https://lists.osuosl.org/mailman/listinfo/intel-wired-lan _______________________________________________ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan