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 C1D57C4332F for ; Mon, 14 Mar 2022 10:08:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238288AbiCNKJZ (ORCPT ); Mon, 14 Mar 2022 06:09:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44452 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232365AbiCNKJY (ORCPT ); Mon, 14 Mar 2022 06:09:24 -0400 Received: from smtp-relay-internal-1.canonical.com (smtp-relay-internal-1.canonical.com [185.125.188.123]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8409B140E2 for ; Mon, 14 Mar 2022 03:08:10 -0700 (PDT) Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-1.canonical.com (Postfix) with ESMTPS id 89C323F5F3 for ; Mon, 14 Mar 2022 10:08:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1647252488; bh=+r4tw3AIRRBCkpVG/g0AlJLi4SN41rvsicv4GMS6cfQ=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=OK20dovGnDlpL81vjd1FpO75ZE7eKWCDRVyBNbgm6MoBBEp7h3lij5QlxI9V7KwMX wxGYZ2Z2u5biU0h6t7VmcqyFlt2CKLVm+qfGDOVsOZ8Wf7Y5FD4PExWt+gqHnQCtok QkLCdjlojQWECMbiRhoOfchA1IP22/4r37c4AveGmNygpLVngFG9qiiGjCdfWpBi4I YHjBChLfBwfObAv7WH84jOjMLcn5TI8oKB7GrUSp5yR5kKDSfSs058TJ1mSz/LUAJf Oa6CK7lOxhftB4UNtV5Hb6ehtSly8PikJFnyrRrfzw37Uz/H/E2NFIYjpJhkW+u2Bq WGSNLhoATetBg== Received: by mail-ed1-f69.google.com with SMTP id da28-20020a056402177c00b00415ce4b20baso8398366edb.17 for ; Mon, 14 Mar 2022 03:08:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=+r4tw3AIRRBCkpVG/g0AlJLi4SN41rvsicv4GMS6cfQ=; b=jHNQSxeCdXufXrCu3lO0yjWeU54x5bGzcN/SBh0lm/36IFThg4oYw32kUs5YX2AxIM nb2feyUBhWe/8uzFQ+VyJqaP27WHKDy5/cJURc5XXxvhOuOI9Ac25OgXih9kDW0DxArl VzECnMWVIagwffmCiRfNmjXrYbkItxTDPbcj887vJThnRW+kFhkvelYvo8wRp8QunVP8 FwwLDzCOLA9rg4LGiQTNm3Ib+DZIBwS8f/RrKZyh1XcxzmGFvCbyKMPbC6MwNCLa2AhN /FSo/KsXbcbe91Zi6MT2v2vrG2symiEMLPJlH2r1nR+K2PYnhAP0FyEBNdmFR0BaPLzK 58uw== X-Gm-Message-State: AOAM533oNDI0bFLj9GmOQzV+gcVvkDwemAlrd6j+7lchWeSrLrq78GPm Qo/MBLHQMmafmOPVkyKUP2Ju/AvhnABytjLx1RWTyBA6VOfZz0xZ78EL6ul3fBHkaZWhXSZ3p8i Uo9aJJReoJjrOf1DTaqmMrv7Bg+DWM9Tdu/BQgXnU9Ko= X-Received: by 2002:a17:906:280b:b0:6ce:f3c7:688f with SMTP id r11-20020a170906280b00b006cef3c7688fmr18204832ejc.468.1647252487972; Mon, 14 Mar 2022 03:08:07 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzI3ZzFyIAPUEjWu3adeD7O9BLeA6APF0933yAfK5HzcfK0RO66XgNxL4F3sH+IGjsRiB8sDw== X-Received: by 2002:a17:906:280b:b0:6ce:f3c7:688f with SMTP id r11-20020a170906280b00b006cef3c7688fmr18204802ejc.468.1647252487681; Mon, 14 Mar 2022 03:08:07 -0700 (PDT) Received: from [192.168.0.152] (xdsl-188-155-174-239.adslplus.ch. [188.155.174.239]) by smtp.googlemail.com with ESMTPSA id k7-20020aa7c047000000b004132d3b60aasm7616899edo.78.2022.03.14.03.08.06 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 14 Mar 2022 03:08:07 -0700 (PDT) Message-ID: Date: Mon, 14 Mar 2022 11:08:05 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH v2 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params bindings Content-Language: en-US To: Pavan Kondeti Cc: Sandeep Maheswaram , Rob Herring , Andy Gross , Bjorn Andersson , Kishon Vijay Abraham I , Vinod Koul , Greg Kroah-Hartman , Wesley Cheng , Stephen Boyd , Doug Anderson , Matthias Kaehlcke , devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-phy@lists.infradead.org, linux-usb@vger.kernel.org, quic_ppratap@quicinc.com, quic_kriskura@quicinc.com References: <1646288011-32242-1-git-send-email-quic_c_sanm@quicinc.com> <1646288011-32242-2-git-send-email-quic_c_sanm@quicinc.com> <20220314032952.GA27561@hu-pkondeti-hyd.qualcomm.com> <20220314081613.GA28402@hu-pkondeti-hyd.qualcomm.com> <20220314094054.GB28402@hu-pkondeti-hyd.qualcomm.com> From: Krzysztof Kozlowski In-Reply-To: <20220314094054.GB28402@hu-pkondeti-hyd.qualcomm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On 14/03/2022 10:40, Pavan Kondeti wrote: > Hi Krzysztof, > > Thanks for your suggestions and guidance on this. > > On Mon, Mar 14, 2022 at 09:36:02AM +0100, Krzysztof Kozlowski wrote: >> On 14/03/2022 09:16, Pavan Kondeti wrote: >>> Hi Krzysztof, >>> >>> On Mon, Mar 14, 2022 at 08:39:57AM +0100, Krzysztof Kozlowski wrote: >>>> On 14/03/2022 04:29, Pavan Kondeti wrote: >>>>> Hi Krzysztof, >>>>> >>>>> On Thu, Mar 03, 2022 at 04:59:22PM +0100, Krzysztof Kozlowski wrote: >>>>>> On 03/03/2022 07:13, Sandeep Maheswaram wrote: >>>>>>> Add device tree bindings for SNPS phy tuning parameters. >>>>>>> >>>>>>> Signed-off-by: Sandeep Maheswaram >>>>>>> --- >>>>>>> .../bindings/phy/qcom,usb-snps-femto-v2.yaml | 125 +++++++++++++++++++++ >>>>>>> 1 file changed, 125 insertions(+) >>>>>>> >>>>>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml >>>>>>> index 0dfe691..227c097 100644 >>>>>>> --- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml >>>>>>> +++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml >>>>>>> @@ -50,6 +50,131 @@ properties: >>>>>>> vdda33-supply: >>>>>>> description: phandle to the regulator 3.3V supply node. >>>>>>> >>>>>>> + qcom,hs-disconnect: >>>>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>>>> + description: >>>>>>> + This adjusts the voltage level for the threshold used to >>>>>>> + detect a disconnect event at the host. Possible values are. >>>>>> >>>>>> ':', instead of full stop. >>>>>> >>>>>>> + 7 -> +21.56% >>>>>>> + 6 -> +17.43% >>>>>>> + 5 -> +13.32% >>>>>>> + 4 -> +9.73% >>>>>>> + 3 -> +6.3 >>>>>>> + 2 -> +3.17% >>>>>>> + 1 -> 0, Design default% >>>>>> >>>>>> Use "default:" instead. Here and in other places. >>>>>> >>>>>>> + 0 -> -2.72% >>>>>> >>>>>> In current form this should be an enum... but actually current form is >>>>>> wrong. You should not store register values in DT. What if next version >>>>>> of hardware has a different meaning of these values? >>>>>> >>>>>> Instead, you should store here meaningful values, not register values. >>>>>> >>>>> >>>>> Thanks for the feedback. >>>>> >>>>> The values in % really makes the tuning easy. People look at the eye diagram >>>>> and decided whether to increase/decrease the margin. The absolute values >>>>> may not be that useful. All we need is an "adjustment" here. The databook >>>>> it self does not give any absolute values. >>>>> >>>>> I agree to the "enum" suggestion which we have been following for the >>>>> qusb2 driver already. >>>>> >>>>> The values have not changed in the last 5 years for this hardware block, so >>>>> defining enums for the % values would be really helpful. >>>> >>>> I did not say you cannot store here percentages. Quite opposite - store >>>> here the percentages. Just do not store register value. No. Please read >>>> my comment again - meaningful values are needed. >>>> >>> >>> IIUC, you are asking us to come up with a meaningful values to encode the >>> percentage values. However, all the % increments are not linear, so we can't >>> come up with {min, max} scheme. Lets take an example of hostdisconnect >>> threshold. >>> >>> As per the data book, >>> >>> + 7 -> +21.56% >>> + 6 -> +17.43% >>> + 5 -> +13.32% >>> + 4 -> +9.73% >>> + 3 -> +6.3 >>> + 2 -> +3.17% >>> + 1 -> 0, Design default% >>> + 0 -> -2.72% >>> >>> so how do we give meaningful values here? Does the below scheme make sense >>> to you? >> >> By "meaningful value" I mean something which has a understandable >> meaning to reader of this code or to hardware designer. For example >> percentage values or some units (ms, ns, Hz, mA, mV). The value used in >> register is not meaningful in that way to us because it has a meaning >> only to the hardware block. Storing register values is more difficult to >> read later, non-portable and non-scalable. >> >>> >>> #define QCOM_SNPS_FEMTO_HS_DISCONNECT_NEG_2P72 (-272) >>> #define QCOM_SNPS_FEMTO_HS_DISCONNECT_DEFAULT 0 >>> #define QCOM_SNPS_FEMTO_HS_DISCONNECT_3P17 317 >>> #define QCOM_SNPS_FEMTO_HS_DISCONNECT_6P3 63 >> >> This is some define in driver, does not look related to bindings. >> >>> In the driver, we have a mapping (which can be per SoC if required in future) >>> that takes these values and convert to the correct values for a given >>> register. >> >> You focus on driver but I am talking here only about bindings. > > I was saying we define those defines in include/dt-bindings/phy/... header and > use it in the device tree and as well in the driver. Ah, I did not get it. That's not the solution for this case. defines in dt-bindings are for constants which already can be in DT, e.g. IDs. Your register values should not be stored in DT. > >> >> What could be the meaningful value? Percentage could work. You have >> there a negative value, so I wonder what type of percentage is it? What >> is the formula? > > I just multiplied by 100 since device tree has no support for floating (as per > my knowledge). The negative value represents it lowers the disconnect > threshold by 2.72% of the default value. if it makes sense, we could also > start from 0 like below. ok > > #define QCOM_SNPS_FEMTO_HS_DISCONNECT_NEG_2P72_PCT 0 > #define QCOM_SNPS_FEMTO_HS_DISCONNECT_DEFAULT 1 > #define QCOM_SNPS_FEMTO_HS_DISCONNECT_3P17_PCT 2 > #define QCOM_SNPS_FEMTO_HS_DISCONNECT_6P3_PCT 3 > > The driver can have a table to map these bindings. This looks much better > than those x100 formula values. Again mention driver how he can map it. I mostly don't care about the driver. :) I think we are getting around the problem, so to emphasize again: do not store register values in the bindings/DT but its meaning, so in your case most likely percentages (or permille or ratio or some other value). Best regards, Krzysztof 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 1338AC433FE for ; Mon, 14 Mar 2022 10:08:23 +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:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Lbv8mzESg+T25r8GCpxRWIItS3vBd/PKiKLwb4Ovo88=; b=qSDBUMBQNTXtqD hbY8VSVw5qJBk/A91f0vFFDNP6UnFgFNIN36dlxcPpkk30XMlq+q5LR7bj0Ml8uVIQo5b4Z9qLDDR wj3g/kSHDQ5UrYIz0Oaa+1/WyFhvxhN+aW4whnBndIpfgZGKa3mOX8QtzkbI9honZbWWzS9Fw0Tfe vLfLsRWVsslTl4fvQ9tON8byT4CfADiOWLcLVdU/tGqzKvhW9LdaF4OJJHf/YT8ZbwfEm7jnfA8Xs UGaeowQEndKzmMmTPhZoKuuNR1DqnAlivzFj9b5f7099z4q2nufVifOBq9OEgUorXHs2BYDZYKkm0 /uaRy8oaZ0LceyQsteCQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nThck-004r6a-Hp; Mon, 14 Mar 2022 10:08:22 +0000 Received: from smtp-relay-internal-1.canonical.com ([185.125.188.123]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nThcg-004r46-Vf for linux-phy@lists.infradead.org; Mon, 14 Mar 2022 10:08:21 +0000 Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-1.canonical.com (Postfix) with ESMTPS id 98B383F79C for ; Mon, 14 Mar 2022 10:08:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1647252488; bh=+r4tw3AIRRBCkpVG/g0AlJLi4SN41rvsicv4GMS6cfQ=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=OK20dovGnDlpL81vjd1FpO75ZE7eKWCDRVyBNbgm6MoBBEp7h3lij5QlxI9V7KwMX wxGYZ2Z2u5biU0h6t7VmcqyFlt2CKLVm+qfGDOVsOZ8Wf7Y5FD4PExWt+gqHnQCtok QkLCdjlojQWECMbiRhoOfchA1IP22/4r37c4AveGmNygpLVngFG9qiiGjCdfWpBi4I YHjBChLfBwfObAv7WH84jOjMLcn5TI8oKB7GrUSp5yR5kKDSfSs058TJ1mSz/LUAJf Oa6CK7lOxhftB4UNtV5Hb6ehtSly8PikJFnyrRrfzw37Uz/H/E2NFIYjpJhkW+u2Bq WGSNLhoATetBg== Received: by mail-ed1-f70.google.com with SMTP id 11-20020a50874b000000b004186b7c1252so2612235edv.3 for ; Mon, 14 Mar 2022 03:08:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=+r4tw3AIRRBCkpVG/g0AlJLi4SN41rvsicv4GMS6cfQ=; b=MN48biM3O97lpMOb5VF2C64XpdsWGAfML4pT+/3pE2qNXuyp6QksPIHazmH+VhNO/9 3Nihftc7+jYJW2XNXeGUnF+A7mRhc4ZtSCyfMpxnVOWfE/WjNXCS8fLh9mwviB5wSikE uHo2bIZCz8aRdPW+o8Hd8VazSQmXuveaMZR7iPZC9CTciMQ8ckKGbRYMyrcfvWJhOwUn pe7yXQ73Th1Fho2l5z8rxt8a5RS6feZCRp4+47rRpiyIFN2PtRHxpevUBnf0TkL9d1uB Y4Eqt1ov6BE4YnlQQZwavqDQQ9D5SOY7vZRQYA5CoD0CCnPPGZKm9RcVAsWZgevYV9U6 iV3w== X-Gm-Message-State: AOAM530Xt7uvBuBF52suzzmf5XaL6xMgAcZXM+5exQDhXRU5CMOXB1S0 bNNB6T6vkjJOnqzjUJ8/k94BzvYx5pnjbd2rvsnUwU/IuJ1WlW7hs1THCzMHfnNOaiMVTxzV3as PaXecGzww+sAaisCfJ8AD94vcvh5eI/E+ShCSUKcEKo4= X-Received: by 2002:a17:906:280b:b0:6ce:f3c7:688f with SMTP id r11-20020a170906280b00b006cef3c7688fmr18204826ejc.468.1647252487963; Mon, 14 Mar 2022 03:08:07 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzI3ZzFyIAPUEjWu3adeD7O9BLeA6APF0933yAfK5HzcfK0RO66XgNxL4F3sH+IGjsRiB8sDw== X-Received: by 2002:a17:906:280b:b0:6ce:f3c7:688f with SMTP id r11-20020a170906280b00b006cef3c7688fmr18204802ejc.468.1647252487681; Mon, 14 Mar 2022 03:08:07 -0700 (PDT) Received: from [192.168.0.152] (xdsl-188-155-174-239.adslplus.ch. [188.155.174.239]) by smtp.googlemail.com with ESMTPSA id k7-20020aa7c047000000b004132d3b60aasm7616899edo.78.2022.03.14.03.08.06 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 14 Mar 2022 03:08:07 -0700 (PDT) Message-ID: Date: Mon, 14 Mar 2022 11:08:05 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH v2 1/3] dt-bindings: phy: qcom, usb-snps-femto-v2: Add phy override params bindings Content-Language: en-US To: Pavan Kondeti Cc: Sandeep Maheswaram , Rob Herring , Andy Gross , Bjorn Andersson , Kishon Vijay Abraham I , Vinod Koul , Greg Kroah-Hartman , Wesley Cheng , Stephen Boyd , Doug Anderson , Matthias Kaehlcke , devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-phy@lists.infradead.org, linux-usb@vger.kernel.org, quic_ppratap@quicinc.com, quic_kriskura@quicinc.com References: <1646288011-32242-1-git-send-email-quic_c_sanm@quicinc.com> <1646288011-32242-2-git-send-email-quic_c_sanm@quicinc.com> <20220314032952.GA27561@hu-pkondeti-hyd.qualcomm.com> <20220314081613.GA28402@hu-pkondeti-hyd.qualcomm.com> <20220314094054.GB28402@hu-pkondeti-hyd.qualcomm.com> From: Krzysztof Kozlowski In-Reply-To: <20220314094054.GB28402@hu-pkondeti-hyd.qualcomm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220314_030819_210411_083EBB67 X-CRM114-Status: GOOD ( 36.51 ) X-BeenThere: linux-phy@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux Phy Mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-phy" Errors-To: linux-phy-bounces+linux-phy=archiver.kernel.org@lists.infradead.org On 14/03/2022 10:40, Pavan Kondeti wrote: > Hi Krzysztof, > > Thanks for your suggestions and guidance on this. > > On Mon, Mar 14, 2022 at 09:36:02AM +0100, Krzysztof Kozlowski wrote: >> On 14/03/2022 09:16, Pavan Kondeti wrote: >>> Hi Krzysztof, >>> >>> On Mon, Mar 14, 2022 at 08:39:57AM +0100, Krzysztof Kozlowski wrote: >>>> On 14/03/2022 04:29, Pavan Kondeti wrote: >>>>> Hi Krzysztof, >>>>> >>>>> On Thu, Mar 03, 2022 at 04:59:22PM +0100, Krzysztof Kozlowski wrote: >>>>>> On 03/03/2022 07:13, Sandeep Maheswaram wrote: >>>>>>> Add device tree bindings for SNPS phy tuning parameters. >>>>>>> >>>>>>> Signed-off-by: Sandeep Maheswaram >>>>>>> --- >>>>>>> .../bindings/phy/qcom,usb-snps-femto-v2.yaml | 125 +++++++++++++++++++++ >>>>>>> 1 file changed, 125 insertions(+) >>>>>>> >>>>>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml >>>>>>> index 0dfe691..227c097 100644 >>>>>>> --- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml >>>>>>> +++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml >>>>>>> @@ -50,6 +50,131 @@ properties: >>>>>>> vdda33-supply: >>>>>>> description: phandle to the regulator 3.3V supply node. >>>>>>> >>>>>>> + qcom,hs-disconnect: >>>>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>>>> + description: >>>>>>> + This adjusts the voltage level for the threshold used to >>>>>>> + detect a disconnect event at the host. Possible values are. >>>>>> >>>>>> ':', instead of full stop. >>>>>> >>>>>>> + 7 -> +21.56% >>>>>>> + 6 -> +17.43% >>>>>>> + 5 -> +13.32% >>>>>>> + 4 -> +9.73% >>>>>>> + 3 -> +6.3 >>>>>>> + 2 -> +3.17% >>>>>>> + 1 -> 0, Design default% >>>>>> >>>>>> Use "default:" instead. Here and in other places. >>>>>> >>>>>>> + 0 -> -2.72% >>>>>> >>>>>> In current form this should be an enum... but actually current form is >>>>>> wrong. You should not store register values in DT. What if next version >>>>>> of hardware has a different meaning of these values? >>>>>> >>>>>> Instead, you should store here meaningful values, not register values. >>>>>> >>>>> >>>>> Thanks for the feedback. >>>>> >>>>> The values in % really makes the tuning easy. People look at the eye diagram >>>>> and decided whether to increase/decrease the margin. The absolute values >>>>> may not be that useful. All we need is an "adjustment" here. The databook >>>>> it self does not give any absolute values. >>>>> >>>>> I agree to the "enum" suggestion which we have been following for the >>>>> qusb2 driver already. >>>>> >>>>> The values have not changed in the last 5 years for this hardware block, so >>>>> defining enums for the % values would be really helpful. >>>> >>>> I did not say you cannot store here percentages. Quite opposite - store >>>> here the percentages. Just do not store register value. No. Please read >>>> my comment again - meaningful values are needed. >>>> >>> >>> IIUC, you are asking us to come up with a meaningful values to encode the >>> percentage values. However, all the % increments are not linear, so we can't >>> come up with {min, max} scheme. Lets take an example of hostdisconnect >>> threshold. >>> >>> As per the data book, >>> >>> + 7 -> +21.56% >>> + 6 -> +17.43% >>> + 5 -> +13.32% >>> + 4 -> +9.73% >>> + 3 -> +6.3 >>> + 2 -> +3.17% >>> + 1 -> 0, Design default% >>> + 0 -> -2.72% >>> >>> so how do we give meaningful values here? Does the below scheme make sense >>> to you? >> >> By "meaningful value" I mean something which has a understandable >> meaning to reader of this code or to hardware designer. For example >> percentage values or some units (ms, ns, Hz, mA, mV). The value used in >> register is not meaningful in that way to us because it has a meaning >> only to the hardware block. Storing register values is more difficult to >> read later, non-portable and non-scalable. >> >>> >>> #define QCOM_SNPS_FEMTO_HS_DISCONNECT_NEG_2P72 (-272) >>> #define QCOM_SNPS_FEMTO_HS_DISCONNECT_DEFAULT 0 >>> #define QCOM_SNPS_FEMTO_HS_DISCONNECT_3P17 317 >>> #define QCOM_SNPS_FEMTO_HS_DISCONNECT_6P3 63 >> >> This is some define in driver, does not look related to bindings. >> >>> In the driver, we have a mapping (which can be per SoC if required in future) >>> that takes these values and convert to the correct values for a given >>> register. >> >> You focus on driver but I am talking here only about bindings. > > I was saying we define those defines in include/dt-bindings/phy/... header and > use it in the device tree and as well in the driver. Ah, I did not get it. That's not the solution for this case. defines in dt-bindings are for constants which already can be in DT, e.g. IDs. Your register values should not be stored in DT. > >> >> What could be the meaningful value? Percentage could work. You have >> there a negative value, so I wonder what type of percentage is it? What >> is the formula? > > I just multiplied by 100 since device tree has no support for floating (as per > my knowledge). The negative value represents it lowers the disconnect > threshold by 2.72% of the default value. if it makes sense, we could also > start from 0 like below. ok > > #define QCOM_SNPS_FEMTO_HS_DISCONNECT_NEG_2P72_PCT 0 > #define QCOM_SNPS_FEMTO_HS_DISCONNECT_DEFAULT 1 > #define QCOM_SNPS_FEMTO_HS_DISCONNECT_3P17_PCT 2 > #define QCOM_SNPS_FEMTO_HS_DISCONNECT_6P3_PCT 3 > > The driver can have a table to map these bindings. This looks much better > than those x100 formula values. Again mention driver how he can map it. I mostly don't care about the driver. :) I think we are getting around the problem, so to emphasize again: do not store register values in the bindings/DT but its meaning, so in your case most likely percentages (or permille or ratio or some other value). Best regards, Krzysztof -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy