diff mbox series

[net-next,v5,5/9] dt-bindings: net: add QCA807x PHY defines

Message ID 20240201151747.7524-6-ansuelsmth@gmail.com (mailing list archive)
State Superseded
Headers show
Series net: phy: Introduce PHY Package concept | expand

Commit Message

Christian Marangi Feb. 1, 2024, 3:17 p.m. UTC
From: Robert Marko <robert.marko@sartura.hr>

Add DT bindings defined for Qualcomm QCA807x PHY series related to
calibration and DAC settings.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 include/dt-bindings/net/qcom-qca807x.h | 30 ++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 include/dt-bindings/net/qcom-qca807x.h

Comments

Krzysztof Kozlowski Feb. 2, 2024, 7:41 a.m. UTC | #1
On 01/02/2024 16:17, Christian Marangi wrote:
> From: Robert Marko <robert.marko@sartura.hr>
> 
> Add DT bindings defined for Qualcomm QCA807x PHY series related to
> calibration and DAC settings.

Nothing from this file is used and your commit msg does not provide
rationale "why", thus it does not look like something for bindings.
Otherwise please point me which patch with *driver* uses these bindings.

> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  include/dt-bindings/net/qcom-qca807x.h | 30 ++++++++++++++++++++++++++

Use filename matching compatible, so vendor,device. No wildcards, unless
your compatible also has them.

>  1 file changed, 30 insertions(+)
>  create mode 100644 include/dt-bindings/net/qcom-qca807x.h
> 



Best regards,
Krzysztof
Christian Marangi Feb. 2, 2024, 3:19 p.m. UTC | #2
On Fri, Feb 02, 2024 at 08:41:56AM +0100, Krzysztof Kozlowski wrote:
> On 01/02/2024 16:17, Christian Marangi wrote:
> > From: Robert Marko <robert.marko@sartura.hr>
> > 
> > Add DT bindings defined for Qualcomm QCA807x PHY series related to
> > calibration and DAC settings.
> 
> Nothing from this file is used and your commit msg does not provide
> rationale "why", thus it does not look like something for bindings.
> Otherwise please point me which patch with *driver* uses these bindings.
>

Hi, since I have to squash this, I will include the reason in the schema
patch.

Anyway these are raw values used to configure the qcom,control-dac
property.

In the driver it's used by qca807x_config_init. We read what is set in
DT and we configure the reg accordingly.

If this is wrong should we use a more schema friendly approach with
declaring an enum of string and document that there?

> > 
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  include/dt-bindings/net/qcom-qca807x.h | 30 ++++++++++++++++++++++++++
> 
> Use filename matching compatible, so vendor,device. No wildcards, unless
> your compatible also has them.
> 
> >  1 file changed, 30 insertions(+)
> >  create mode 100644 include/dt-bindings/net/qcom-qca807x.h
> > 
> 
> 
> 
> Best regards,
> Krzysztof
>
Conor Dooley Feb. 2, 2024, 4:58 p.m. UTC | #3
On Fri, Feb 02, 2024 at 04:19:15PM +0100, Christian Marangi wrote:
> On Fri, Feb 02, 2024 at 08:41:56AM +0100, Krzysztof Kozlowski wrote:
> > On 01/02/2024 16:17, Christian Marangi wrote:
> > > From: Robert Marko <robert.marko@sartura.hr>
> > > 
> > > Add DT bindings defined for Qualcomm QCA807x PHY series related to
> > > calibration and DAC settings.
> > 
> > Nothing from this file is used and your commit msg does not provide
> > rationale "why", thus it does not look like something for bindings.
> > Otherwise please point me which patch with *driver* uses these bindings.
> >
> 
> Hi, since I have to squash this, I will include the reason in the schema
> patch.
> 
> Anyway these are raw values used to configure the qcom,control-dac
> property.

Maybe I am missing something, but a quick scan of the patchset and a
grep of the tree doesn't show this property being documented anywhere.

> In the driver it's used by qca807x_config_init. We read what is set in
> DT and we configure the reg accordingly.
> 
> If this is wrong should we use a more schema friendly approach with
> declaring an enum of string and document that there?

Without any idea of what that property is used for it is hard to say,
but personally I much prefer enums of strings for what looks like a
property that holds register values.

That you felt it necessary to add defines for the values makes it more
like that that is the case.

Cheers,
Conor.

> 
> > > 
> > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > ---
> > >  include/dt-bindings/net/qcom-qca807x.h | 30 ++++++++++++++++++++++++++
> > 
> > Use filename matching compatible, so vendor,device. No wildcards, unless
> > your compatible also has them.
> > 
> > >  1 file changed, 30 insertions(+)
> > >  create mode 100644 include/dt-bindings/net/qcom-qca807x.h
> > > 
> > 
> > 
> > 
> > Best regards,
> > Krzysztof
> > 
> 
> -- 
> 	Ansuel
Christian Marangi Feb. 2, 2024, 5:03 p.m. UTC | #4
On Fri, Feb 02, 2024 at 04:58:32PM +0000, Conor Dooley wrote:
> On Fri, Feb 02, 2024 at 04:19:15PM +0100, Christian Marangi wrote:
> > On Fri, Feb 02, 2024 at 08:41:56AM +0100, Krzysztof Kozlowski wrote:
> > > On 01/02/2024 16:17, Christian Marangi wrote:
> > > > From: Robert Marko <robert.marko@sartura.hr>
> > > > 
> > > > Add DT bindings defined for Qualcomm QCA807x PHY series related to
> > > > calibration and DAC settings.
> > > 
> > > Nothing from this file is used and your commit msg does not provide
> > > rationale "why", thus it does not look like something for bindings.
> > > Otherwise please point me which patch with *driver* uses these bindings.
> > >
> > 
> > Hi, since I have to squash this, I will include the reason in the schema
> > patch.
> > 
> > Anyway these are raw values used to configure the qcom,control-dac
> > property.
> 
> Maybe I am missing something, but a quick scan of the patchset and a
> grep of the tree doesn't show this property being documented anywhere.
> 
> > In the driver it's used by qca807x_config_init. We read what is set in
> > DT and we configure the reg accordingly.
> > 
> > If this is wrong should we use a more schema friendly approach with
> > declaring an enum of string and document that there?
> 
> Without any idea of what that property is used for it is hard to say,
> but personally I much prefer enums of strings for what looks like a
> property that holds register values.
> 
> That you felt it necessary to add defines for the values makes it more
> like that that is the case.
>

Ok, no problem. The big idea here was really to skip having to have a
big function that parse strings but I get that it's better handle with
string and have them documented directly in the yaml.

> > 
> > > > 
> > > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > > ---
> > > >  include/dt-bindings/net/qcom-qca807x.h | 30 ++++++++++++++++++++++++++
> > > 
> > > Use filename matching compatible, so vendor,device. No wildcards, unless
> > > your compatible also has them.
> > > 
> > > >  1 file changed, 30 insertions(+)
> > > >  create mode 100644 include/dt-bindings/net/qcom-qca807x.h
> > > > 
> > > 
> > > 
> > > 
> > > Best regards,
> > > Krzysztof
> > > 
> > 
> > -- 
> > 	Ansuel
diff mbox series

Patch

diff --git a/include/dt-bindings/net/qcom-qca807x.h b/include/dt-bindings/net/qcom-qca807x.h
new file mode 100644
index 000000000000..e7d4d09b7dd4
--- /dev/null
+++ b/include/dt-bindings/net/qcom-qca807x.h
@@ -0,0 +1,30 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
+/*
+ * Device Tree constants for the Qualcomm QCA807X PHYs
+ */
+
+#ifndef _DT_BINDINGS_QCOM_QCA807X_H
+#define _DT_BINDINGS_QCOM_QCA807X_H
+
+/* Full amplitude, full bias current */
+#define QCA807X_CONTROL_DAC_FULL_VOLT_BIAS		0
+/* Amplitude follow DSP (amplitude is adjusted based on cable length), half bias current */
+#define QCA807X_CONTROL_DAC_DSP_VOLT_HALF_BIAS		1
+/* Full amplitude, bias current follow DSP (bias current is adjusted based on cable length) */
+#define QCA807X_CONTROL_DAC_FULL_VOLT_DSP_BIAS		2
+/* Both amplitude and bias current follow DSP */
+#define QCA807X_CONTROL_DAC_DSP_VOLT_BIAS		3
+/* Full amplitude, half bias current */
+#define QCA807X_CONTROL_DAC_FULL_VOLT_HALF_BIAS		4
+/* Amplitude follow DSP setting; 1/4 bias current when cable<10m,
+ * otherwise half bias current
+ */
+#define QCA807X_CONTROL_DAC_DSP_VOLT_QUARTER_BIAS	5
+/* Full amplitude; same bias current setting with “010” and “011”,
+ * but half more bias is reduced when cable <10m
+ */
+#define QCA807X_CONTROL_DAC_FULL_VOLT_HALF_BIAS_SHORT	6
+/* Amplitude follow DSP; same bias current setting with “110”, default value */
+#define QCA807X_CONTROL_DAC_DSP_VOLT_HALF_BIAS_SHORT	7
+
+#endif