mbox series

[0/4] Add support for Qualcomm QCA807x PHYs

Message ID 20201222222637.3204929-1-robert.marko@sartura.hr
Headers show
Series Add support for Qualcomm QCA807x PHYs | expand

Message

Robert Marko Dec. 22, 2020, 10:26 p.m. UTC
This patch series adds support for Qualcomm QCA807x PHYs.

These are really common companion PHYs on boards featuring
Qualcomm IPQ40xx, IPQ60xx and IPQ807x SoCs.

They are 2 or 5 port IEEE 802.3 clause 22 compliant
10BASE-Te, 100BASE-TX and 1000BASE-T PHY-s.

They feature 2 SerDes, one for PSGMII or QSGMII connection with MAC,
while second one is SGMII for connection to MAC or fiber.

Both models have a combo port that supports 1000BASE-X and 100BASE-FX
fiber.

Each PHY inside of QCA807x series has 2 digitally controlled output only
pins that natively drive LED-s.
But some vendors used these to driver generic LED-s controlled by
user space, so lets enable registering each PHY as GPIO controller and
add driver for it.

Robert Marko (4):
  dt-bindings: net: Add QCA807x PHY
  dt-bindings: net: Add bindings for Qualcomm QCA807x
  net: phy: Add Qualcomm QCA807x driver
  MAINTAINERS: Add entry for Qualcomm QCA807x PHY driver

 .../devicetree/bindings/net/qcom,qca807x.yaml |  88 ++
 MAINTAINERS                                   |   9 +
 drivers/net/phy/Kconfig                       |  10 +
 drivers/net/phy/Makefile                      |   1 +
 drivers/net/phy/qca807x.c                     | 811 ++++++++++++++++++
 include/dt-bindings/net/qcom-qca807x.h        |  45 +
 6 files changed, 964 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/qcom,qca807x.yaml
 create mode 100644 drivers/net/phy/qca807x.c
 create mode 100644 include/dt-bindings/net/qcom-qca807x.h

Comments

Andrew Lunn Dec. 23, 2020, 12:56 a.m. UTC | #1
> +  gpio-controller: true
> +  "#gpio-cells":
> +    const: 2
> +
> +  qcom,single-led-1000:
> +    description: |
> +      If present, then dedicated 1000 Mbit will light up for 1000Base-T.
> +      This is a workround for boards with a single LED instead of two.
> +    type: boolean
> +
> +  qcom,single-led-100:
> +    description: |
> +      If present, then dedicated 1000 Mbit will light up for 100Base-TX.
> +      This is a workround for boards with a single LED instead of two.
> +    type: boolean
> +
> +  qcom,single-led-10:
> +    description: |
> +      If present, then dedicated 1000 Mbit will light up for 10Base-Te.
> +      This is a workround for boards with a single LED instead of two.
> +    type: boolean

Sorry, but no. Please look at the work being done for allow PHY LEDs
to be controlled via the LED subsystem. 

> +  qcom,tx-driver-strength:
> +    description: PSGMII/QSGMII TX driver strength control.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]

Please use the actual values here, and have the driver convert to the
value poked into the register. So the property would be
qcom,tx-driver-strength-mv and it would have the value 220 for
example.

> +
> +  qcom,control-dac:
> +    description: Analog MDI driver amplitude and bias current.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2, 3, 4, 5, 6, 7]

Make here.

     Andrew
Rob Herring (Arm) Jan. 3, 2021, 4:46 p.m. UTC | #2
On Wed, Dec 23, 2020 at 01:56:33AM +0100, Andrew Lunn wrote:
> > +  gpio-controller: true
> > +  "#gpio-cells":
> > +    const: 2
> > +
> > +  qcom,single-led-1000:
> > +    description: |
> > +      If present, then dedicated 1000 Mbit will light up for 1000Base-T.
> > +      This is a workround for boards with a single LED instead of two.
> > +    type: boolean
> > +
> > +  qcom,single-led-100:
> > +    description: |
> > +      If present, then dedicated 1000 Mbit will light up for 100Base-TX.
> > +      This is a workround for boards with a single LED instead of two.
> > +    type: boolean
> > +
> > +  qcom,single-led-10:
> > +    description: |
> > +      If present, then dedicated 1000 Mbit will light up for 10Base-Te.
> > +      This is a workround for boards with a single LED instead of two.
> > +    type: boolean
> 
> Sorry, but no. Please look at the work being done for allow PHY LEDs
> to be controlled via the LED subsystem. 
> 
> > +  qcom,tx-driver-strength:
> > +    description: PSGMII/QSGMII TX driver strength control.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
> 
> Please use the actual values here, and have the driver convert to the
> value poked into the register. So the property would be
> qcom,tx-driver-strength-mv and it would have the value 220 for
> example.

The LED binding has properties for specifying the current already. And 
it's max current which is the h/w property where as anything less is 
just software configuration (IOW, doesn't belong in DT).
Robert Marko Jan. 7, 2021, 4:39 p.m. UTC | #3
On Wed, Dec 23, 2020 at 1:56 AM Andrew Lunn <andrew@lunn.ch> wrote:
>

> > +  gpio-controller: true

> > +  "#gpio-cells":

> > +    const: 2

> > +

> > +  qcom,single-led-1000:

> > +    description: |

> > +      If present, then dedicated 1000 Mbit will light up for 1000Base-T.

> > +      This is a workround for boards with a single LED instead of two.

> > +    type: boolean

> > +

> > +  qcom,single-led-100:

> > +    description: |

> > +      If present, then dedicated 1000 Mbit will light up for 100Base-TX.

> > +      This is a workround for boards with a single LED instead of two.

> > +    type: boolean

> > +

> > +  qcom,single-led-10:

> > +    description: |

> > +      If present, then dedicated 1000 Mbit will light up for 10Base-Te.

> > +      This is a workround for boards with a single LED instead of two.

> > +    type: boolean

>

> Sorry, but no. Please look at the work being done for allow PHY LEDs

> to be controlled via the LED subsystem.


Ok, I will drop the LED configuration from the driver until there is a generic
way to do it.
I was following the work on it, but it seems to have halted after September.
>

> > +  qcom,tx-driver-strength:

> > +    description: PSGMII/QSGMII TX driver strength control.

> > +    $ref: /schemas/types.yaml#/definitions/uint32

> > +    enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]

>

> Please use the actual values here, and have the driver convert to the

> value poked into the register. So the property would be

> qcom,tx-driver-strength-mv and it would have the value 220 for

> example.

Ok, it actually makes more sense than using dt-binding includes for this.
>

> > +

> > +  qcom,control-dac:

> > +    description: Analog MDI driver amplitude and bias current.

> > +    $ref: /schemas/types.yaml#/definitions/uint32

> > +    enum: [0, 1, 2, 3, 4, 5, 6, 7]

>

> Make here.

I am using defines in dt-binding includes for this one as it makes the
values humanly readable in DT as these configure the amplitude and
bias current for power saving.
>

>      Andrew
Robert Marko Jan. 7, 2021, 4:41 p.m. UTC | #4
On Sun, Jan 3, 2021 at 6:00 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > > +  qcom,tx-driver-strength:
> > > > +    description: PSGMII/QSGMII TX driver strength control.
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
> > >
> > > Please use the actual values here, and have the driver convert to the
> > > value poked into the register. So the property would be
> > > qcom,tx-driver-strength-mv and it would have the value 220 for
> > > example.
> >
> > The LED binding has properties for specifying the current already. And
> > it's max current which is the h/w property where as anything less is
> > just software configuration (IOW, doesn't belong in DT).
>
> Hi Rob
>
> My understanding of this is it is the drive strength of the SERDES
> line. Nothing to do with LEDs. The description needs improving.

Yes, this is used to set the PSGMII/QSMII SerDes TX driver strength.
It has nothing to do with LEDs.
I agree that the description is a bit confusing, I will try to make it
a bit clearer.
>
>       Andrew