[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<LV3PR11MB874233C067E17B585571948AEC49A@LV3PR11MB8742.namprd11.prod.outlook.com>
Date: Wed, 9 Jul 2025 23:02:34 +0000
From: <Tristram.Ha@...rochip.com>
To: <maxime.chevallier@...tlin.com>
CC: <Woojung.Huh@...rochip.com>, <andrew@...n.ch>, <olteanv@...il.com>,
<robh@...nel.org>, <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
<davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
<pabeni@...hat.com>, <marex@...x.de>, <UNGLinuxDriver@...rochip.com>,
<devicetree@...r.kernel.org>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net-next 6/6 v2] net: dsa: microchip: Setup fiber ports
for KSZ8463
> Hi Tristram
>
> On Tue, 8 Jul 2025 19:45:44 +0000
> <Tristram.Ha@...rochip.com> wrote:
>
> > > Hi Tristram,
> > >
> > > On Mon, 7 Jul 2025 20:16:48 -0700
> > > <Tristram.Ha@...rochip.com> wrote:
> > >
> > > > From: Tristram Ha <tristram.ha@...rochip.com>
> > > >
> > > > The fiber ports in KSZ8463 cannot be detected internally, so it requires
> > > > specifying that condition in the device tree. Like the one used in
> > > > Micrel PHY the port link can only be read and there is no write to the
> > > > PHY. The driver programs registers to operate fiber ports correctly.
> > > >
> > > > The PTP function of the switch is also turned off as it may interfere the
> > > > normal operation of the MAC.
> > > >
> > > > Signed-off-by: Tristram Ha <tristram.ha@...rochip.com>
> > > > ---
> > > > drivers/net/dsa/microchip/ksz8.c | 26 ++++++++++++++++++++++++++
> > > > drivers/net/dsa/microchip/ksz_common.c | 3 +++
> > > > 2 files changed, 29 insertions(+)
> > > >
> > > > diff --git a/drivers/net/dsa/microchip/ksz8.c b/drivers/net/dsa/microchip/ksz8.c
> > > > index 904db68e11f3..1207879ef80c 100644
> > > > --- a/drivers/net/dsa/microchip/ksz8.c
> > > > +++ b/drivers/net/dsa/microchip/ksz8.c
> > > > @@ -1715,6 +1715,7 @@ void ksz8_config_cpu_port(struct dsa_switch *ds)
> > > > const u32 *masks;
> > > > const u16 *regs;
> > > > u8 remote;
> > > > + u8 fiber_ports = 0;
> > > > int i;
> > > >
> > > > masks = dev->info->masks;
> > > > @@ -1745,6 +1746,31 @@ void ksz8_config_cpu_port(struct dsa_switch *ds)
> > > > else
> > > > ksz_port_cfg(dev, i, regs[P_STP_CTRL],
> > > > PORT_FORCE_FLOW_CTRL, false);
> > > > + if (p->fiber)
> > > > + fiber_ports |= (1 << i);
> > > > + }
> > > > + if (ksz_is_ksz8463(dev)) {
> > > > + /* Setup fiber ports. */
> > >
> > > What does fiber port mean ? Is it 100BaseFX ? As this configuration is
> > > done only for the CPU port (it seems), looks like this mode is planned
> > > to be used as the MAC to MAC mode on the DSA conduit. So, instead of
> > > using this property maybe you should implement that as handling the
> > > "100base-x" phy-mode ?
> > >
> > > > + if (fiber_ports) {
> > > > + regmap_update_bits(ksz_regmap_16(dev),
> > > > + reg16(dev, KSZ8463_REG_CFG_CTRL),
> > > > + fiber_ports << PORT_COPPER_MODE_S,
> > > > + 0);
> > > > + regmap_update_bits(ksz_regmap_16(dev),
> > > > + reg16(dev, KSZ8463_REG_DSP_CTRL_6),
> > > > + COPPER_RECEIVE_ADJUSTMENT, 0);
> > > > + }
> > > > +
> > > > + /* Turn off PTP function as the switch's proprietary way of
> > > > + * handling timestamp is not supported in current Linux PTP
> > > > + * stack implementation.
> > > > + */
> > > > + regmap_update_bits(ksz_regmap_16(dev),
> > > > + reg16(dev, KSZ8463_PTP_MSG_CONF1),
> > > > + PTP_ENABLE, 0);
> > > > + regmap_update_bits(ksz_regmap_16(dev),
> > > > + reg16(dev, KSZ8463_PTP_CLK_CTRL),
> > > > + PTP_CLK_ENABLE, 0);
> > > > }
> > > > }
> > > >
> > > > diff --git a/drivers/net/dsa/microchip/ksz_common.c
> > > b/drivers/net/dsa/microchip/ksz_common.c
> > > > index c08e6578a0df..b3153b45ced9 100644
> > > > --- a/drivers/net/dsa/microchip/ksz_common.c
> > > > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > > > @@ -5441,6 +5441,9 @@ int ksz_switch_register(struct ksz_device *dev)
> > > > &dev->ports[port_num].interface);
> > > >
> > > > ksz_parse_rgmii_delay(dev, port_num, port);
> > > > + dev->ports[port_num].fiber =
> > > > + of_property_read_bool(port,
> > > > + "micrel,fiber-mode");
> > >
> > > Shouldn't this be described in the binding ?
> > >
> > > > }
> > > > of_node_put(ports);
> > > > }
> >
> > The "micrel,fiber-mode" is described in Documentation/devicetree/
> > bindings/net/micrel.txt.
>
> Yes but that's for PHYs right ? Yours is under the DSA "ports"
> node.
>
> >
> > Some old KSZ88XX switches have option of using fiber in a port running
> > 100base-fx. Typically they have a register indicating that configuration
> > and the driver just treats the port as having a PHY and reads the link
> > status and speed as normal except there is no write to those PHY related
> > registers. KSZ8463 does not have that option so the driver needs to be
> > told.
>
> That's what I understood from your comments indeed, what thew me off
> guard is that all ports's fiber mode is configured in the
> config_cpu_port() callback.
>
> I'd like to one day be able to deprecate these
> micrel,fiber-mode/ti,fiber-mode properties in favor of the ports API
> that's being worked on, but I guess we can roll with it for now.
Is that required to declare the parameter in the device tree document?
It is just a bit of hassle to create another one just for KSZ8463. The
other old switches likely do not need this. And the new KSZ9477 and
LAN937X switches use the SFP case logic.
Powered by blists - more mailing lists