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 295DCC7EE23 for ; Fri, 26 May 2023 10:40:05 +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=8+zWWj2qwLzeEtsFHirkJBBIWHLCXsCGmydBTBOKVP0=; b=DaYdCjBrpFjhFy J/3kz7lD7vK+hfHcX2I1bRZcz/vYguepFiOPBAsMmmzWx8xImvYvZPVWMztzVJTOO4Sl19B+JwcXz A22OrgeYyQRTOCiXt2RSJo2XxqkqTsZj1tbbTGI3Rm3W9u8yt41hGa+ORrsA4vCwXg1lmreGKcqHa 3eP204YkQXQID5UZTfyAfLVx8MlsBduyLO0tqoTWT4JkN7S6yXIM0jtbIqHFo1NiwRYY1NBHI+yry UG5hBns4y1sTRseQ6FENgAZfostwBrFHKzt7cx+77t09AOfdhVXWNzvoBMgMpIgbDVA/0aI37JBSr KT79KtnH0ToYGYYDWVgg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q2UrB-0023su-0V; Fri, 26 May 2023 10:39:37 +0000 Received: from mail-ed1-x52a.google.com ([2a00:1450:4864:20::52a]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q2Ur7-0023qK-08 for linux-arm-kernel@lists.infradead.org; Fri, 26 May 2023 10:39:35 +0000 Received: by mail-ed1-x52a.google.com with SMTP id 4fb4d7f45d1cf-51144dddd4cso778663a12.1 for ; Fri, 26 May 2023 03:39:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=resnulli-us.20221208.gappssmtp.com; s=20221208; t=1685097570; x=1687689570; 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=nnaGXZy0EI8Syg5zP9M4YNVm4y2Oe3wo0VJ6UyZ1uIw=; b=zxrhNc1mJUsS4852rPJmlSFlhzK0Pcla7b/5zqYZ6cP7hUIgnNPSF/qXPFuIUiKhZp YeS10qFFVqQ5SqMADLMxsDLx/4MGQV/DSQrGea9mBB0pti04AZdXYLyj/KBIYa/EMni3 +EO0Plwxs7gEJrdTbGS0po0d3jTanw8Pxfd24Jg87/kbA++olBDUHGhNy0NOSqRRkspp 1C/tiX1Ag8k71mR9y2E9YXf2ABey+KPfyIia7AkyMKeEW1F8sEQi1vOvcKv+zJCPN7fr /uvnFID3VJUquh2wrIYO9iGVSAuyRjZBpch/PorT1Q+eSMgXAnoJerB2IobjDMHciQFR qXcw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685097570; x=1687689570; 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=nnaGXZy0EI8Syg5zP9M4YNVm4y2Oe3wo0VJ6UyZ1uIw=; b=dwmcEuEwERfu1OwM7hs9GA9aD/0RpkkETsnn3sv0q6u6HDXcfymVYa5LzzEtpSPG+f xKKg/Q53A+BqENFuoT6uEAa0YMeFgaY8epl9Omhazf2CUWrK5RRGMsuT4K0HgR7IyHp2 5X6vQzLPEVwHisKPrXBZ0Hjs9bmnb3zR6jrcPhsZA/GUqKL0zvZTAtdfZfhmE9Y5vqeP Lq0lSfWscbJVI8WN0VkAqiLWl0N16cah6jATpsxgdWnJmS//qbVR6UmOVUywChxv3SME fQpiMbtPrR5bsw6Nw0xYq1rBmfpr2ihWK61mJKsux//tNkG4kvpF/KsA5Z9iQ/Br5AUC IhSQ== X-Gm-Message-State: AC+VfDziZT0CnoNG9QhplxzHyXHA596ZKiCYiAGy6KFC//pWupvrt/5m yowP8mrBBq64dTAwngGQOpJNsg== X-Google-Smtp-Source: ACHHUZ7QzX0ywzRsxsFjcM6NqyJniGVWg4W4KK8SfdbnzGuSeMSCR+HCckHhxmEXu8FOMB75MTSN4Q== X-Received: by 2002:aa7:d8cd:0:b0:510:f1dc:86c0 with SMTP id k13-20020aa7d8cd000000b00510f1dc86c0mr937375eds.32.1685097569882; Fri, 26 May 2023 03:39:29 -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 z6-20020aa7c646000000b00501d73cfc86sm1426697edr.9.2023.05.26.03.39.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 26 May 2023 03:39:29 -0700 (PDT) Date: Fri, 26 May 2023 12:39:28 +0200 From: Jiri Pirko To: "Kubalewski, Arkadiusz" Cc: Vadim Fedorenko , Jakub Kicinski , Jonathan Lemon , Paolo Abeni , "Olech, Milena" , "Michalik, Michal" , "linux-arm-kernel@lists.infradead.org" , Vadim Fedorenko , poros , mschmidt , "netdev@vger.kernel.org" , "linux-clk@vger.kernel.org" Subject: Re: [RFC PATCH v7 0/8] Create common DPLL configuration API Message-ID: References: <20230428002009.2948020-1-vadfed@meta.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230526_033933_078333_5AC538B3 X-CRM114-Status: GOOD ( 24.23 ) 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 Fri, May 26, 2023 at 12:14:00PM CEST, arkadiusz.kubalewski@intel.com wrote: >>From: Jiri Pirko >>Sent: Wednesday, May 17, 2023 12:19 PM >> >>Let me summarize the outcome of the discussion between me and Jakub >>regarding attributes, handles and ID lookups in the RFCv7 thread: >> >>-------------------------------------------------------------- >>** Needed changes for RFCv8 ** >> >>1) No scoped indexes. >> The indexes passed from driver to dpll core during call of: >> dpll_device_get() - device_idx >> dpll_pin_get() - pin_idx >> should be for INTERNAL kernel use only and NOT EXPOSED over uapi. >> Therefore following attributes need to be removed: >> DPLL_A_PIN_IDX >> DPLL_A_PIN_PARENT_IDX >> > >Seems doable. >So just to be clear, configuring a pin-pair (MUXed pins) will now be done >with DPLL_A_PIN_PARENT nested attribute. >I.e. configuring state of pin on parent: >DPLL_CMD_PIN_SET > DPLL_A_PIN_ID (id of pin being configured) > DPLL_A_PIN_PARENT (nested) > DPLL_A_PIN_ID (id of parent pin) > DPLL_A_PIN_STATE(expected state) > ... (other pin-pair attributes to be set) > >Is that ok, or we need separated attribute like DPLL_A_PIN_PARENT_ID?? >I think there is no need for separated one, documentation shall just reflect >that. >Also we have nested attribute DPLL_A_DEVICE which is used to show connections >between PIN and multiple dpll devices, to make it consistent I will rename >it to `DPLL_A_DEVICE_PARENT` and make configuration set cmd for the pin-dpll >pair similar to the above: >DPLL_CMD_PIN_SET > DPLL_A_PIN_ID (id of pin being configured) > DPLL_A_DEVICE_PARENT (nested) It is a parent of pin, not device. The name is confusing. But see below. > DPLL_A_ID (id of parent dpll) > DPLL_A_PIN_STATE(expected state) > ... (other pin-dpll attributes to be set) > >Does it make sense? Yeah, good idea. I like this. We will have consistent approach for parent pin and device. To take it even further, we can have one nested attr for parent and decide the parent type according to the id attr given: DPLL_CMD_PIN_SET DPLL_A_PIN_ID (id of pin being configured) DPLL_A_PIN_PARENT (nested) DPLL_A_PIN_ID (id of parent pin) DPLL_A_PIN_STATE(expected state) ... (other pin-pair attributes to be set) DPLL_CMD_PIN_SET DPLL_A_PIN_ID (id of pin being configured) DPLL_A_PIN_PARENT (nested) DPLL_A_ID (id of parent dpll) DPLL_A_PIN_STATE(expected state) ... (other pin-dpll attributes to be set) Same for PIN_GET Makes sense? > > >>2) For device, the handle will be DPLL_A_ID == dpll->id. >> This will be the only handle for device for every >> device related GET, SET command and every device related notification. >> Note: this ID is not deterministing and may be different depending on >> order of device probes etc. >> > >Seems doable. > >>3) For pin, the handle will be DPLL_A_PIN_ID == pin->id >> This will be the only handle for pin for every >> pin related GET, SET command and every pin related notification. >> Note: this ID is not deterministing and may be different depending on >> order of device probes etc. >> > >Seems doable. > >>4) Remove attribute: >> DPLL_A_PIN_LABEL >> and replace it with: >> DPLL_A_PIN_PANEL_LABEL (string) >> DPLL_A_PIN_XXX (string) >> where XXX is a label type, like for example: >> DPLL_A_PIN_BOARD_LABEL >> DPLL_A_PIN_BOARD_TRACE >> DPLL_A_PIN_PACKAGE_PIN >> > >Sorry, I don't get this idea, what are those types? >What are they for? The point is to make the driver developer to think before passing randomly constructed label strings. For example, "board_label" would lead the developer to check how the pin is labeled on the board. The "panel_label" indicates this is label on a panel. Also, developer can fill multiple labels for the same pin. > >>5) Make sure you expose following attributes for every device and >> pin GET/DUMP command reply message: >> DPLL_A_MODULE_NAME >> DPLL_A_CLOCK_ID >> > >Seems doable. > >>6) Remove attributes: >> DPLL_A_DEV_NAME >> DPLL_A_BUS_NAME >> as they no longer have any value and do no make sense (even in RFCv7) >> > >Seems doable. > >> >>-------------------------------------------------------------- >>** Lookup commands ** >> >>Basically these would allow user to query DEVICE_ID and PIN_ID >>according to provided atributes (see examples below). >> >>These would be from my perspective optional for this patchsets. >>I believe we can do it as follow-up if needed. For example for mlx5 >>I don't have usecase for it, since I can consistently get PIN_ID >>using RT netlink for given netdev. But I can imagine that for non-SyncE >>dpll driver this would make sense to have. >> >>1) Introduce CMD_GET_ID - query the kernel for a dpll device >> specified by given attrs >> Example: >> -> DPLL_A_MODULE_NAME >> DPLL_A_CLOCK_ID >> DPLL_A_TYPE >> <- DPLL_A_ID >> Example: >> -> DPLL_A_MODULE_NAME >> DPLL_A_CLOCK_ID >> <- DPLL_A_ID >> Example: >> -> DPLL_A_MODULE_NAME >> <- -EINVAL (Extack: "multiple devices matched") >> >> If user passes a subset of attrs which would not result in >> a single match, kernel returns -EINVAL and proper extack message. >> > >Seems ok. > >>2) Introduce CMD_GET_PIN_ID - query the kernel for a dpll pin >> specified by given attrs >> Example: >> -> DPLL_A_MODULE_NAME >> DPLL_A_CLOCK_ID >> DPLL_A_PIN_TYPE >> DPLL_A_PIN_PANEL_LABEL >> <- DPLL_A_PIN_ID >> Example: >> -> DPLL_A_MODULE_NAME >> DPLL_A_CLOCK_ID >> <- DPLL_A_PIN_ID (There was only one pin for given module/clock_id) >> Example: >> -> DPLL_A_MODULE_NAME >> DPLL_A_CLOCK_ID >> <- -EINVAL (Extack: "multiple pins matched") >> >> If user passes a subset of attrs which would not result in >> a single match, kernel returns -EINVAL and proper extack message. > > >Seems ok. > >Will try to implement those now. Cool, thx! > >Thank you, >Arkadiusz _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel