mbox series

[net-next,v1,0/4] net: phy: mxl-gpy: broken interrupt fixes

Message ID 20221202151204.3318592-1-michael@walle.cc
Headers show
Series net: phy: mxl-gpy: broken interrupt fixes | expand

Message

Michael Walle Dec. 2, 2022, 3:12 p.m. UTC 
The GPY215 has a broken interrupt pin. This patch series tries to
workaround that and because in general that is not possible, disables the
interrupts by default and falls back to polling mode. There is an opt-in
via the devicetree.

net vs net-next: I'm not sure. No one seems to have noticed it so far.
My board I care about has no support for older kernel. Apart from that,
the first patch might be for net. The last one would need a new device
tree property, so it might only apply for net-next? Also it will disable
interrupts by default now.
Let me know what you think. I can send the first patch independently with a
Fixes tag and resend the last ones after the merge window. (The last one
depends on the first).

Btw. I just noticed that this series won't apply cleanly, because it
references patch context changed by
https://lore.kernel.org/netdev/20221202144900.3298204-1-michael@walle.cc/
:(

Michael Walle (4):
  net: phy: mxl-gpy: add MDINT workaround
  dt-bindings: vendor-prefixes: add MaxLinear
  dt-bindings: net: phy: add MaxLinear GPY2xx bindings
  net: phy: mxl-gpy: disable interrupts on GPY215 by default

 .../bindings/net/maxlinear,gpy2xx.yaml        | 47 ++++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 drivers/net/phy/mxl-gpy.c                     | 88 +++++++++++++++++++
 3 files changed, 137 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/maxlinear,gpy2xx.yaml

Comments

Michael Walle Dec. 2, 2022, 10:53 p.m. UTC | #1 
Am 2022-12-02 19:23, schrieb Andrew Lunn:
> On Fri, Dec 02, 2022 at 04:12:01PM +0100, Michael Walle wrote:
>> At least the GPY215B and GPY215C has a bug where it is still driving 
>> the
>> interrupt line (MDINT) even after the interrupt status register is 
>> read
>> and its bits are cleared. This will cause an interrupt storm.
>> 
>> Although the MDINT is multiplexed with a GPIO pin and theoretically we
>> could switch the pinmux to GPIO input mode, this isn't possible 
>> because
>> the access to this register will stall exactly as long as the 
>> interrupt
>> line is asserted. We exploit this very fact and just read a random
>> internal register in our interrupt handler. This way, it will be 
>> delayed
>> until the external interrupt line is released and an interrupt storm 
>> is
>> avoided.
>> 
>> The internal register access via the mailbox was deduced by looking at
>> the downstream PHY API because the datasheet doesn't mention any of
>> this.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>>  drivers/net/phy/mxl-gpy.c | 83 
>> +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 83 insertions(+)
>> 
>> diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
>> index 0ff7ef076072..20e610dda891 100644
>> --- a/drivers/net/phy/mxl-gpy.c
>> +++ b/drivers/net/phy/mxl-gpy.c
>> @@ -9,6 +9,7 @@
>>  #include <linux/module.h>
>>  #include <linux/bitfield.h>
>>  #include <linux/hwmon.h>
>> +#include <linux/mutex.h>
>>  #include <linux/phy.h>
>>  #include <linux/polynomial.h>
>>  #include <linux/netdevice.h>
>> @@ -81,6 +82,14 @@
>>  #define VSPEC1_TEMP_STA	0x0E
>>  #define VSPEC1_TEMP_STA_DATA	GENMASK(9, 0)
>> 
>> +/* Mailbox */
>> +#define VSPEC1_MBOX_DATA	0x5
>> +#define VSPEC1_MBOX_ADDRLO	0x6
>> +#define VSPEC1_MBOX_CMD		0x7
>> +#define VSPEC1_MBOX_CMD_ADDRHI	GENMASK(7, 0)
>> +#define VSPEC1_MBOX_CMD_RD	(0 << 8)
>> +#define VSPEC1_MBOX_CMD_READY	BIT(15)
>> +
>>  /* WoL */
>>  #define VPSPEC2_WOL_CTL		0x0E06
>>  #define VPSPEC2_WOL_AD01	0x0E08
>> @@ -88,7 +97,15 @@
>>  #define VPSPEC2_WOL_AD45	0x0E0A
>>  #define WOL_EN			BIT(0)
>> 
>> +/* Internal registers, access via mbox */
>> +#define REG_GPIO0_OUT		0xd3ce00
>> +
>>  struct gpy_priv {
>> +	struct phy_device *phydev;
>> +
>> +	/* serialize mailbox acesses */
>> +	struct mutex mbox_lock;
>> +
> 
>>  static int gpy_probe(struct phy_device *phydev)
>>  {
>>  	struct device *dev = &phydev->mdio.dev;
>> @@ -228,7 +286,9 @@ static int gpy_probe(struct phy_device *phydev)
>>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>  	if (!priv)
>>  		return -ENOMEM;
>> +	priv->phydev = phydev;
> 
> I don't think you use this anywhere. Maybe in one of the following
> patches?

Arg. Yes, it's an leftover from when I was using a workqueue to
reenable the interrupts again.

Any opinion whether this patch should be net or net-next?

-michael
Andrew Lunn Dec. 3, 2022, 8:41 p.m. UTC | #2 
On Fri, Dec 02, 2022 at 04:12:00PM +0100, Michael Walle wrote:
> The GPY215 has a broken interrupt pin. This patch series tries to
> workaround that and because in general that is not possible, disables the
> interrupts by default and falls back to polling mode. There is an opt-in
> via the devicetree.
> 
> net vs net-next: I'm not sure. No one seems to have noticed it so far.

I guess you could fail to notice 2ms of interrupt storm, if it only
happens on link down. But we should probably fix it. So please post
the first patch to net with a Fixes: tag. The rest to net-next.

    Andrew