| Message ID | 20210210125523.2146352-1-robert.marko@sartura.hr |
|---|---|
| Headers | show |
| Series | Add support for Qualcomm QCA807x PHYs | expand |
On Wed, Feb 10, 2021 at 01:55:22PM +0100, Robert Marko wrote:
Hi Robert
Could you move the GPIO driver into a patch of its own, and Cc: the
GPIO maintainer and list for that patch please.
Andrew
> +static int qca807x_psgmii_config(struct phy_device *phydev) > +{ > + struct device_node *node = phydev->mdio.dev.of_node; > + int psgmii_az, tx_amp, ret = 0; > + u32 tx_driver_strength_dt; > + > + /* Workaround to enable AZ transmitting ability */ > + if (of_property_read_bool(node, "qcom,psgmii-az")) { > + psgmii_az = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, PSGMII_MODE_CTRL); > + psgmii_az &= ~PSGMII_MODE_CTRL_AZ_WORKAROUND_MASK; > + psgmii_az |= FIELD_PREP(PSGMII_MODE_CTRL_AZ_WORKAROUND_MASK, 0xc); > + ret = phy_write_mmd(phydev, MDIO_MMD_PMAPMD, PSGMII_MODE_CTRL, psgmii_az); > + psgmii_az = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, PSGMII_MODE_CTRL); > + } > + > + /* PSGMII/QSGMII TX amp set to DT defined value instead of default 600mV */ > + if (!of_property_read_u32(node, "qcom,tx-driver-strength", &tx_driver_strength_dt)) { > + int tx_driver_strength; > + > + switch (tx_driver_strength_dt) { > + case 140: > + tx_driver_strength = 0; > + break; > + case 160: > + tx_driver_strength = 1; > + break; > + case 180: > + tx_driver_strength = 2; > + break; > + case 200: > + tx_driver_strength = 3; > + break; > + case 220: > + tx_driver_strength = 4; > + break; > + case 240: > + tx_driver_strength = 5; > + break; > + case 260: > + tx_driver_strength = 6; > + break; > + case 280: > + tx_driver_strength = 7; > + break; > + case 300: > + tx_driver_strength = 8; > + break; > + case 320: > + tx_driver_strength = 9; > + break; > + case 400: > + tx_driver_strength = 10; > + break; > + case 500: > + tx_driver_strength = 11; > + break; > + case 600: > + tx_driver_strength = 12; > + break; > + default: > + tx_driver_strength = 12; > + break; Please return -EINVAL here. The value in DT is not valid, so you should error out. If there is no value at all, then it is O.K. to default to 12. Andrew
On Wed, Feb 10, 2021 at 01:55:20PM +0100, Robert Marko wrote: > Add DT bindings for Qualcomm QCA807x PHY series. > > Signed-off-by: Robert Marko <robert.marko@sartura.hr> > Cc: Luka Perkov <luka.perkov@sartura.hr> > --- > Changes in v2: > * Drop PSGMII/QSGMII TX driver defines > > include/dt-bindings/net/qcom-qca807x.h | 30 ++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > create mode 100644 include/dt-bindings/net/qcom-qca807x.h > > diff --git a/include/dt-bindings/net/qcom-qca807x.h b/include/dt-bindings/net/qcom-qca807x.h > new file mode 100644 > index 000000000000..a5ac12777c2b > --- /dev/null > +++ b/include/dt-bindings/net/qcom-qca807x.h > @@ -0,0 +1,30 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * 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 Are these really properties of the board? That is what device tree is supposed to be about. These seem like configuration options. Which suggests they should actually be a PHY tunable. Andrew