Re: [net-next PATCH 12/13] drivers: net: dsa: qca8k: add support for pws config reg

Anyway the difference is that they made 2 different package version of
the qca8327. One with 176 pin and one with 148 pin.

I assume they have different product numbers. So you can quote them in
the DT binding? Are they BGP or QFP? Can somebody easily count the
pins?

quoted

quoted

+
+	if (of_property_read_bool(node, "qca,power-on-sel"))
+		val |= QCA8K_PWS_POWER_ON_SEL;

What happens if you unconditionally do this? Why is a DT property
required?

This is needed to bypass the power on strapping and use the regs config.
The switch can use hardware pin to set eeprom presence and leds open
drain. Setting this bit on bypass the hardware strapping and sets these
2 thing based on the regs.

So first off, it sounds like you have the DT property named wrong. It
should be 'qca,ignore-power-on-sel'.

However, why do you even need this? Generally, strapping gives you the
defaults. Registers get loaded with a value determined by the
strapping. But after that, you can change the value, based on
additional information. Or are you saying the register is read only
when strapping is used?

quoted

quoted

+	if (of_property_read_bool(node, "qca,led-open-drain"))
+		/* POWER_ON_SEL needs to be set when configuring led to open drain */
+		val |= QCA8K_PWS_LED_OPEN_EN_CSR | QCA8K_PWS_POWER_ON_SEL;

At minimum, you need to clearly document that qca,led-open-drain
implies 'qca,ignore-power-on-sel'. I would probably go further and
return -EINVAL if qca,led-open-drain is set and
'qca,ignore-power-on-sel' is not.

	Andrew