2022-08-30 11:12:03

by Arun Ramadoss

Subject: [RFC Patch net-next v3 1/3] net: dsa: microchip: use dev_ops->reset instead of exit in ksz_switch_register

ksz8_switch_exit, ksz9477_switch_exit and lan937x_switch_exit functions
all call the reset function which is assigned to dev_ops->reset hooks.
So instead of calling the dev_ops->exit in ksz_switch_register during
the error condition of dsa_register_switch, dev_ops->reset is used now.
The dev_ops->exit can be extended in lan937x for freeing up the irq
during the ksz_spi_remove. If we add the irq remove in the exit function
and it is called during the dsa_switch_register error condition, kernel
panic happens since irq is setup only in setup operation. To avoid the
kernel panic, dev_ops->reset is used instead of exit.

Signed-off-by: Arun Ramadoss <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 6bd69a7e6809..da9bdf753f7a 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1963,7 +1963,7 @@ int ksz_switch_register(struct ksz_device *dev)

ret = dsa_register_switch(dev->ds);
if (ret) {
- dev->dev_ops->exit(dev);
+ dev->dev_ops->reset(dev);
return ret;
}

--
2.36.1

2022-08-30 12:54:20

by Andrew Lunn

Subject: Re: [RFC Patch net-next v3 1/3] net: dsa: microchip: use dev_ops->reset instead of exit in ksz_switch_register

On Tue, Aug 30, 2022 at 04:23:01PM +0530, Arun Ramadoss wrote:
> ksz8_switch_exit, ksz9477_switch_exit and lan937x_switch_exit functions
> all call the reset function which is assigned to dev_ops->reset hooks.
> So instead of calling the dev_ops->exit in ksz_switch_register during
> the error condition of dsa_register_switch, dev_ops->reset is used now.
> The dev_ops->exit can be extended in lan937x for freeing up the irq
> during the ksz_spi_remove. If we add the irq remove in the exit function
> and it is called during the dsa_switch_register error condition, kernel
> panic happens since irq is setup only in setup operation. To avoid the
> kernel panic, dev_ops->reset is used instead of exit.
>
> Signed-off-by: Arun Ramadoss <[email protected]>
> ---
> drivers/net/dsa/microchip/ksz_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 6bd69a7e6809..da9bdf753f7a 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -1963,7 +1963,7 @@ int ksz_switch_register(struct ksz_device *dev)
>
> ret = dsa_register_switch(dev->ds);
> if (ret) {
> - dev->dev_ops->exit(dev);
> + dev->dev_ops->reset(dev);
> return ret;

I don't like this change. You are supposed to be undoing whatever
ksz_switch_register() has done so far. It appears to of called
ops->init() so far, so calling ops->exit() should be the opposite.

If you are making exit() not the opposite of init() in later patches,
it suggests your code structure is wrong. If exit() is not already the
opposite of init() then fix that.

Andrew