On Tue, Aug 30, 2022 at 06:05:46PM +0200, Oleksij Rempel wrote:

On Tue, Aug 30, 2022 at 12:58:30PM +0300, Vladimir Oltean wrote:

quoted

Hello,

On Tue, Aug 30, 2022 at 08:15:59AM +0000, Arun.Ramadoss@microchip.com wrote:

...

quoted

quoted

Hi Oleksij,
Is this Bug related to fix in 
https://lore.kernel.org/lkml/20220829105810.577903823@linuxfoundation.org/ (local)
. 
It is observed in ksz8794 switch. I think after applying this bug fix
patch it should work. I don't have ksz8 series to test. I ran the
regression only for ksz9 series switches. 

I find it unlikely that the cited patch will fix a NULL pointer
dereference in ksz_get_gbit(). But rather, some pointer to a structure
is NULL, and we then dereference a member located at its offset 0x5, no?

My eyes are on this:

	const u8 *bitval = dev->info->xmii_ctrl1;

		data8 |= FIELD_PREP(P_GMII_1GBIT_M, bitval[P_GMII_NOT_1GBIT]);
							   ~~~~~~~~~~~~~~~~
							   this is coincidentally
							   also 5

ack.

quoted

See, looking at the struct ksz_chip_data[] array element for KSZ8873
that Oleksij mentions as broken, I do not see xmii_ctrl1 and xmii_ctrl2
as being pointers to anything.

	[KSZ8830] = {
		.chip_id = KSZ8830_CHIP_ID,
		.dev_name = "KSZ8863/KSZ8873",
		.num_vlans = 16,
		.num_alus = 0,
		.num_statics = 8,
		.cpu_ports = 0x4,	/* can be configured as cpu port */
		.port_cnt = 3,
		.ops = &ksz8_dev_ops,
		.mib_names = ksz88xx_mib_names,
		.mib_cnt = ARRAY_SIZE(ksz88xx_mib_names),
		.reg_mib_cnt = MIB_COUNTER_NUM,
		.regs = ksz8863_regs,
		.masks = ksz8863_masks,
		.shifts = ksz8863_shifts,
		.supports_mii = {false, false, true},
		.supports_rmii = {false, false, true},
		.internal_phy = {true, true, false},
	},

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.

-- 
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 |