lkml.org 
[lkml]   [2022]   [Aug]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [Patch net-next v2 0/9] net: dsa: microchip: add support for phylink mac config and link up
On Wed, Aug 31, 2022 at 06:18:59PM +0300, Vladimir Oltean wrote:
> On Wed, Aug 31, 2022 at 09:43:24AM +0200, Oleksij Rempel wrote:
> > > > Should we point them to ksz8795_xmii_ctrl0 and ksz8795_xmii_ctrl1? I don't know.
> > > > Could you find out what these should be set to?
> > >
> > > xmii_ctrl0/1 are missing and it make no sense to add it.
> > > KSZ8873 switch is controlling CPU port MII configuration over global,
> > > not port based register.
> > >
> > > I'll better define separate ops for this chip.
> >
> > Hm, not only KSZ8830/KSZ8863/KSZ8873 are affected. ksz8795 compatible
> > series with defined .xmii_ctrl0/.xmii_ctrl1 are broken too. Because it
> > is writing to the global config register over ksz_pwrite8 function. It
> > means, we are writing to 0xa6 instead of 0x06. And to 0xf6 instead of
> > 0x56.
>
> What do you mean by "global config register"? The Is_1Gbps bit is still
> part of a port register, it's just that this particular register is only
> defined for the 5th port (port #4, the only xMII port on KSZ7895 AFAIU).
> That doesn't necessarily make it a "global" register.
>
> Datasheet says:
>
> Register 22 (0x16): Reserved
> Register 38 (0x26): Reserved
> Register 54 (0x36): Reserved
> Register 70 (0x46): Reserved
> Register 86 (0x56): Port 5 Interface Control 6
>
> I wonder if it's ok to modify the regs table like this, because we
> should then only touch P_XMII_CTRL_1 using port 4:
>
> static const u16 ksz8795_regs[] = {
> [REG_IND_CTRL_0] = 0x6E,
> [REG_IND_DATA_8] = 0x70,
> [REG_IND_DATA_CHECK] = 0x72,
> [REG_IND_DATA_HI] = 0x71,
> [REG_IND_DATA_LO] = 0x75,
> [REG_IND_MIB_CHECK] = 0x74,
> [REG_IND_BYTE] = 0xA0,
> [P_FORCE_CTRL] = 0x0C,
> [P_LINK_STATUS] = 0x0E,
> [P_LOCAL_CTRL] = 0x07,
> [P_NEG_RESTART_CTRL] = 0x0D,
> [P_REMOTE_STATUS] = 0x08,
> [P_SPEED_STATUS] = 0x09,
> [S_TAIL_TAG_CTRL] = 0x0C,
> [P_STP_CTRL] = 0x02,
> [S_START_CTRL] = 0x01,
> [S_BROADCAST_CTRL] = 0x06,
> [S_MULTICAST_CTRL] = 0x04,
> [P_XMII_CTRL_0] = 0x06,
> - [P_XMII_CTRL_1] = 0x56,
> + [P_XMII_CTRL_1] = 0x06,
> };
>

Speed configuration on ksz8795 is done over two registers:
Register 86 (0x56): Port 5 Interface Control 6: Is_1Gbps - BIT(6)
and
Register 6 (0x06): Global Control 4: Switch SW5-MII/RMII Speed -BIT(4)

both are accessed on wrong offsets.

I would prefer to do following steps:
- remove everything from ksz_phylink_mac_link_up() except of
dev->dev_ops->phylink_mac_link_up
- move ksz_duplex_flowctrl(), ksz_port_set_xmii_speed()... to ksz9477.c
and rename them. Assign ksz9477_phylink_mac_link_up()
dev->dev_ops->phylink_mac_link_up
- create separate function ksz8795_phylink_mac_link_up()
- use documented, not generic register names.

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

\
 
 \ /
  Last update: 2022-08-31 18:12    [W:0.094 / U:0.256 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site