Linux-Doc Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH net-next 6/9] net: phy: add backplane kr driver support
@ 2020-04-01 13:35 Florinel Iordache
  2020-04-01 13:51 ` Andrew Lunn
  0 siblings, 1 reply; 18+ messages in thread
From: Florinel Iordache @ 2020-04-01 13:35 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: davem, netdev, f.fainelli, hkallweit1, linux, devicetree,
	linux-doc, robh+dt, mark.rutland, kuba, corbet, shawnguo, Leo Li,
	Madalin Bucur (OSS),
	Ioana Ciornei, linux-kernel, Florinel Iordache

> On Thu, Mar 26, 2020 at 03:51:19PM +0200, Florinel Iordache wrote:
> > +static void setup_supported_linkmode(struct phy_device *bpphy) {
> > +     struct backplane_phy_info *bp_phy = bpphy->priv;
> 
> I'm not sure it is a good idea to completely take over phydev->priv like this, in
> what is just helper code. What if the PHY driver needs memory of its own? There
> are a few examples of this already in other PHY drivers. Could a KR PHY contain
> a temperature sensor? Could it contain statistics counters which need
> accumulating?
> 
>         Andrew

Backplane KR driver allocates memory for structure backplane_phy_info
which is saved in phydev->priv. After all this is the purpose of priv
according to its description in phy.h: <<private data pointer For use
by PHYs to maintain extra state>>. Here the priv is used to maintain
extra state needed for backplane. This way the backplane specific data
becomes available for all PHY callbacks (defined in struct phy_driver)
that receive a pointer to phy_device structure. This initial version
doesn't include accumulating statistics counters but we have in plan
to add these in future versions. The counters will be kept in specific
structures as members of the main backplane data mentioned above
and entire support will be integrated with ethtool.

Florin.

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH net-next 6/9] net: phy: add backplane kr driver support
@ 2020-04-24 12:14 Florinel Iordache
  0 siblings, 0 replies; 18+ messages in thread
From: Florinel Iordache @ 2020-04-24 12:14 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn
  Cc: davem, netdev, hkallweit1, linux, devicetree, linux-doc, robh+dt,
	mark.rutland, kuba, corbet, shawnguo, Leo Li, Madalin Bucur (OSS),
	Ioana Ciornei, linux-kernel

> On 3/26/2020 6:07 PM, Andrew Lunn wrote:
> >> +static u32 le_ioread32(void __iomem *reg) {
> >> +    return ioread32(reg);
> >> +}
> >> +
> >> +static void le_iowrite32(u32 value, void __iomem *reg) {
> >> +    iowrite32(value, reg);
> >> +}
> >> +
> >> +static u32 be_ioread32(void __iomem *reg) {
> >> +    return ioread32be(reg);
> >> +}
> >> +
> >> +static void be_iowrite32(u32 value, void __iomem *reg) {
> >> +    iowrite32be(value, reg);
> >> +}
> >
> > This is very surprising to me. I've not got my head around the
> > structure of this code yet, but i'm surprised to see memory mapped
> > access functions in generic code.
> 
> This abstraction makes no sense whatsoever, you already have
> io{read,write}32{be,} to deal with the correct endian, and you can use the
> standard Device Tree properties 'big-endian', 'little-endian', 'native-endian' to
> decide which of those of to use. If you need to introduce a wrapper or indirect
> function calls to select the correct I/O accessor, that is fine of course.
> --
> Florian

Hi Florian,
I need these wrappers in generic code in order to automatically assign the proper
I/O accessor in the following structure according to endianness specified in DT.

/* Endianness specific memory I/O */
struct mem_io {
	u32 (*read32)(void __iomem *addr);
	void (*write32)(u32 value, void __iomem *addr);
};

And then the usage is straightforward in device specific code:
io.read32(&reg_base->tcsr3) ...
io.write32((io.read32(&reg_base->tcsr1) ... 

without the need to check endianness at each call and select which read/write function to use.
This is done in order to reduce the overall number of LOC (lines of code).

Florin.

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH net-next 6/9] net: phy: add backplane kr driver support
@ 2020-04-01 14:01 Florinel Iordache
  2020-04-01 14:11 ` Andrew Lunn
  2020-04-01 14:14 ` Russell King - ARM Linux admin
  0 siblings, 2 replies; 18+ messages in thread
From: Florinel Iordache @ 2020-04-01 14:01 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: davem, netdev, f.fainelli, hkallweit1, linux, devicetree,
	linux-doc, robh+dt, mark.rutland, kuba, corbet, shawnguo, Leo Li,
	Madalin Bucur (OSS),
	Ioana Ciornei, linux-kernel, Florinel Iordache

> On Thu, Mar 26, 2020 at 03:51:19PM +0200, Florinel Iordache wrote:
> > Add support for backplane kr generic driver including link training
> > (ieee802.3ap/ba) and fixed equalization algorithm
> >
> > Signed-off-by: Florinel Iordache <florinel.iordache@nxp.com>
> > +/* Read AN Link Status */
> > +static int is_an_link_up(struct phy_device *bpphy) {
> > +     struct backplane_phy_info *bp_phy = bpphy->priv;
> > +     int ret, val = 0;
> > +
> > +     mutex_lock(&bp_phy->bpphy_lock);
> > +
> > +     /* Read twice because Link_Status is LL (Latched Low) bit */
> > +     val = phy_read_mmd(bpphy, MDIO_MMD_AN, bp_phy-
> >bp_dev.mdio.an_status);
> > +     val = phy_read_mmd(bpphy, MDIO_MMD_AN,
> > + bp_phy->bp_dev.mdio.an_status);
> 
> Why not just
> 
> val = phy_read_mmd(bpphy, MDIO_MMD_AN, MDIO_CTRL1);
> 
> Or is your hardware not actually conformant to the standard?
> 
> There has also been a lot of discussion of reading the status twice is correct or
> not. Don't you care the link has briefly gone down and up again?
> 
>         Andrew

This could be changed to use directly the MDIO_STAT1 in order to read
AN status (and use MDIO_CTRL1 for writing the control register) but this
is more flexible and more readable since we defined the structure
kr_mdio_info that contains all registers offsets required by backplane
driver like: LT(link training) registers, AN registers, PMD registers.
So we wanted to put all these together to be clear that all these
offsets are essential for backplane driver and they can be setup
automatically by calling the function: backplane_setup_mdio_c45.

+ void backplane_setup_mdio_c45(struct backplane_kr_info *bpkr)
+ /* KX/KR AN registers: IEEE802.3 Clause 45 (MMD 7) */
+ bpkr->mdio.an_control = MDIO_CTRL1;
+ bpkr->mdio.an_status = MDIO_STAT1;
+ bpkr->mdio.an_ad_ability_0 = MDIO_PMA_EXTABLE_10GBKR;
+ bpkr->mdio.an_ad_ability_1 = MDIO_PMA_EXTABLE_10GBKR + 1;
+ bpkr->mdio.an_lp_base_page_ability_1 = MDIO_PMA_EXTABLE_10GBKR + 4;

This approach is more flexible because it lets open the possibility for
extension on other non-standard devices (devices non-compliant with
clause 45) to still use this driver for backplane operation.
These non-standard devices will have just to define their particular
registers offsets in structure kr_mdio_info and then the rest of the driver
can be used without other modifications.

Florin.

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH net-next 0/9] net: ethernet backplane support
@ 2020-03-26 13:51 Florinel Iordache
       [not found] ` <1585230682-24417-7-git-send-email-florinel.iordache@nxp.com>
  0 siblings, 1 reply; 18+ messages in thread
From: Florinel Iordache @ 2020-03-26 13:51 UTC (permalink / raw)
  To: davem, netdev, andrew, f.fainelli, hkallweit1, linux
  Cc: devicetree, linux-doc, robh+dt, mark.rutland, kuba, corbet,
	shawnguo, leoyang.li, madalin.bucur, ioana.ciornei, linux-kernel,
	Florinel Iordache

Add support for Ethernet Backplane KR generic driver using link training
(ieee802.3ap/ba standards), equalization algorithms (bee, fixed) and
enable qoriq family of devices

Florinel Iordache (9):
  doc: net: add backplane documentation
  dt-bindings: net: add backplane dt bindings
  net: phy: add support for kr phy connection type
  net: fman: add kr support for dpaa1 mac
  net: dpaa2: add kr support for dpaa2 mac
  net: phy: add backplane kr driver support
  net: phy: enable qoriq backplane support
  net: phy: add bee algorithm for kr training
  arm64: dts: add serdes and mdio description

 .../bindings/net/ethernet-controller.yaml          |    3 +-
 .../devicetree/bindings/net/ethernet-phy.yaml      |   53 +
 Documentation/devicetree/bindings/net/serdes.yaml  |   90 ++
 Documentation/networking/backplane.rst             |  165 ++
 arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi     |   33 +-
 arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi     |   97 +-
 arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi     |  160 +-
 arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi     |  128 +-
 .../boot/dts/freescale/qoriq-fman3-0-10g-0.dtsi    |    5 +-
 .../boot/dts/freescale/qoriq-fman3-0-10g-1.dtsi    |    5 +-
 drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c   |   10 +-
 drivers/net/ethernet/freescale/fman/mac.c          |   10 +-
 drivers/net/phy/Kconfig                            |    2 +
 drivers/net/phy/Makefile                           |    1 +
 drivers/net/phy/backplane/Kconfig                  |   40 +
 drivers/net/phy/backplane/Makefile                 |   12 +
 drivers/net/phy/backplane/backplane.c              | 1538 +++++++++++++++++++
 drivers/net/phy/backplane/backplane.h              |  262 ++++
 drivers/net/phy/backplane/eq_bee.c                 | 1078 +++++++++++++
 drivers/net/phy/backplane/eq_fixed.c               |   83 +
 drivers/net/phy/backplane/equalization.h           |  282 ++++
 drivers/net/phy/backplane/link_training.c          | 1604 ++++++++++++++++++++
 drivers/net/phy/backplane/link_training.h          |   34 +
 drivers/net/phy/backplane/qoriq_backplane.c        |  442 ++++++
 drivers/net/phy/backplane/qoriq_backplane.h        |   33 +
 drivers/net/phy/backplane/qoriq_serdes_10g.c       |  470 ++++++
 drivers/net/phy/backplane/qoriq_serdes_28g.c       |  533 +++++++
 drivers/net/phy/phylink.c                          |   15 +-
 include/linux/phy.h                                |    6 +-
 29 files changed, 7176 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/serdes.yaml
 create mode 100644 Documentation/networking/backplane.rst
 create mode 100644 drivers/net/phy/backplane/Kconfig
 create mode 100644 drivers/net/phy/backplane/Makefile
 create mode 100644 drivers/net/phy/backplane/backplane.c
 create mode 100644 drivers/net/phy/backplane/backplane.h
 create mode 100644 drivers/net/phy/backplane/eq_bee.c
 create mode 100644 drivers/net/phy/backplane/eq_fixed.c
 create mode 100644 drivers/net/phy/backplane/equalization.h
 create mode 100644 drivers/net/phy/backplane/link_training.c
 create mode 100644 drivers/net/phy/backplane/link_training.h
 create mode 100644 drivers/net/phy/backplane/qoriq_backplane.c
 create mode 100644 drivers/net/phy/backplane/qoriq_backplane.h
 create mode 100644 drivers/net/phy/backplane/qoriq_serdes_10g.c
 create mode 100644 drivers/net/phy/backplane/qoriq_serdes_28g.c

-- 
1.9.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, back to index

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-01 13:35 [PATCH net-next 6/9] net: phy: add backplane kr driver support Florinel Iordache
2020-04-01 13:51 ` Andrew Lunn
2020-04-01 14:21   ` [EXT] " Florinel Iordache
  -- strict thread matches above, loose matches on Subject: below --
2020-04-24 12:14 Florinel Iordache
2020-04-01 14:01 Florinel Iordache
2020-04-01 14:11 ` Andrew Lunn
2020-04-01 14:14 ` Russell King - ARM Linux admin
2020-03-26 13:51 [PATCH net-next 0/9] net: ethernet backplane support Florinel Iordache
     [not found] ` <1585230682-24417-7-git-send-email-florinel.iordache@nxp.com>
2020-03-26 18:53   ` [PATCH net-next 6/9] net: phy: add backplane kr driver support David Miller
2020-03-26 18:55     ` Joe Perches
2020-03-26 19:07       ` David Miller
2020-03-26 19:42         ` Joe Perches
2020-03-27  1:07   ` Andrew Lunn
2020-03-27 17:43     ` Florian Fainelli
2020-03-27 14:22   ` Andrew Lunn
2020-03-27 18:25     ` Joe Perches
2020-03-27 14:28   ` Andrew Lunn
2020-03-27 14:33   ` Andrew Lunn
2020-03-27 14:38   ` Andrew Lunn

Linux-Doc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-doc/0 linux-doc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-doc linux-doc/ https://lore.kernel.org/linux-doc \
		linux-doc@vger.kernel.org
	public-inbox-index linux-doc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-doc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git