Re: [RFC PATCH 0/4] Add support for QMC HDLC and PHY

From: Andrew Lunn
Date: Thu Apr 13 2023 - 08:48:47 EST


On Thu, Mar 23, 2023 at 11:31:50AM +0100, Herve Codina wrote:
> Hi,
>
> I have a system where I need to handle an HDLC interface.
>
> The HDLC data are transferred using a TDM bus on which a PEF2256 is
> present. The PEF2256 transfers data from/to the TDM bus to/from E1 line.
> This PEF2256 is also connected to a PowerQUICC SoC for the control path
> and the TDM is connected to the SoC (QMC component) for the data path.
>
> From the HDLC driver, I need to handle data using the QMC and carrier
> detection using the PEF2256 (E1 line carrier).
>
> The HDLC driver consider the PEF2256 as a generic PHY.
> So, the design is the following:
>
> +----------+ +-------------+ +---------+
> | HDLC drv | <-data-> | QMC channel | <-- TDM --> | PEF2256 |
> +----------+ +-------------+ | | <--> E1
> ^ +---------+ +---------+ | |
> +-> | Gen PHY | <-> | PEF2256 | <- local bus -> | |
> +---------+ | PHY drv | +---------+
> +---------+

Hi Herver

Sorry, i'm late to the conversation. I'm looking at this from two
different perspectives. I help maintain Ethernet PHYs. And i have
hacked on the IDT 82P2288 E1/T1/J1 framer.

I think there is a block missing from this diagram. There appears to
be an MFD driver for the PEF2256? At least, i see an include for
linux/mfd/pef2256.h.

When i look at the 'phy' driver, i don't see anything a typical PHY
driver used for networking would have. A networking PHY driver often
has the ability to change between modes, like SGMII, QSGMII, 10GBASER.
The equivalent here would be changing between E1, T1 and J1. It has
the ability to change the speed, 1G, 2.5G, 10G etc. This could be
implied via the mode, E1 is 2.048Mbps, T1 1.544Mbps, and i forget what
J1 is. The PEF2256 also seems to support E1/T1/J1. How is its modes
configured?

In fact, this PHY driver does not seem to do any configuration of any
sort on the framer. All it seems to be doing is take notification from
one chain and send them out another chain!

I also wounder if this get_status() call is sufficient. Don't you also
want Red, Yellow and Blue alarms? It is not just the carrier is down,
but why it is down.

Overall, i don't see why you want a PHY. What value does it add?

Andrew