| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGm1_ks-Kw2AtNCNpng6Tp2v3wS=HK7DGWTD0iFcMVMhASSO5A@mail.gmail.com>
Date: Tue, 29 Apr 2014 09:29:52 +0200
From: Yegor Yefremov <yegorslists@...glemail.com>
To: Marc Kleine-Budde <mkl@...gutronix.de>
Cc: netdev <netdev@...r.kernel.org>,
David Miller <davem@...emloft.net>,
"linux-can@...r.kernel.org" <linux-can@...r.kernel.org>,
kernel@...gutronix.de, Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 02/26] can: c_can: Fix startup logic
On Fri, Apr 25, 2014 at 12:00 AM, Marc Kleine-Budde <mkl@...gutronix.de> wrote:
> From: Thomas Gleixner <tglx@...utronix.de>
>
> c_can_start() enables interrupts way too early. The first enabling
> happens when setting the control mode in c_can_chip_config() and then
> again at the end of the function.
>
> But that happens before napi_enable() and that means that an interrupt
> which comes in will disable interrupts again and call napi_schedule,
> which ignores the request and the later napi_enable() is not making
> thinks work either. So the interface is up with all device interrupts
> disabled.
>
> Move the device interrupt after napi_enable() and add it to the other
> callsites of c_can_start() in c_can_set_mode() and c_can_power_up()
>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> Tested-by: Alexander Stein <alexander.stein@...tec-electronic.com>
> Signed-off-by: Marc Kleine-Budde <mkl@...gutronix.de>
Tested-by: Yegor Yefremov <yegorslists@...glemail.com>
> ---
> drivers/net/can/c_can/c_can.c | 35 +++++++++++++++++------------------
> 1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index a5c8dcf..b1629a4 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -612,30 +612,22 @@ static int c_can_chip_config(struct net_device *dev)
> struct c_can_priv *priv = netdev_priv(dev);
>
> /* enable automatic retransmission */
> - priv->write_reg(priv, C_CAN_CTRL_REG,
> - CONTROL_ENABLE_AR);
> + priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_ENABLE_AR);
>
> if ((priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) &&
> (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)) {
> /* loopback + silent mode : useful for hot self-test */
> - priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
> - CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> - priv->write_reg(priv, C_CAN_TEST_REG,
> - TEST_LBACK | TEST_SILENT);
> + priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST);
> + priv->write_reg(priv, C_CAN_TEST_REG, TEST_LBACK | TEST_SILENT);
> } else if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> /* loopback mode : useful for self-test function */
> - priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
> - CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> + priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST);
> priv->write_reg(priv, C_CAN_TEST_REG, TEST_LBACK);
> } else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
> /* silent mode : bus-monitoring mode */
> - priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
> - CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> + priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_TEST);
> priv->write_reg(priv, C_CAN_TEST_REG, TEST_SILENT);
> - } else
> - /* normal mode*/
> - priv->write_reg(priv, C_CAN_CTRL_REG,
> - CONTROL_EIE | CONTROL_SIE | CONTROL_IE);
> + }
>
> /* configure message objects */
> c_can_configure_msg_objects(dev);
> @@ -662,9 +654,6 @@ static int c_can_start(struct net_device *dev)
> /* reset tx helper pointers */
> priv->tx_next = priv->tx_echo = 0;
>
> - /* enable status change, error and module interrupts */
> - c_can_enable_all_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> -
> return 0;
> }
>
> @@ -681,6 +670,7 @@ static void c_can_stop(struct net_device *dev)
>
> static int c_can_set_mode(struct net_device *dev, enum can_mode mode)
> {
> + struct c_can_priv *priv = netdev_priv(dev);
> int err;
>
> switch (mode) {
> @@ -689,6 +679,8 @@ static int c_can_set_mode(struct net_device *dev, enum can_mode mode)
> if (err)
> return err;
> netif_wake_queue(dev);
> + /* enable status change, error and module interrupts */
> + c_can_enable_all_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> break;
> default:
> return -EOPNOTSUPP;
> @@ -1184,6 +1176,8 @@ static int c_can_open(struct net_device *dev)
> can_led_event(dev, CAN_LED_EVENT_OPEN);
>
> napi_enable(&priv->napi);
> + /* enable status change, error and module interrupts */
> + c_can_enable_all_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> netif_start_queue(dev);
>
> return 0;
> @@ -1281,6 +1275,7 @@ int c_can_power_up(struct net_device *dev)
> u32 val;
> unsigned long time_out;
> struct c_can_priv *priv = netdev_priv(dev);
> + int ret;
>
> if (!(dev->flags & IFF_UP))
> return 0;
> @@ -1307,7 +1302,11 @@ int c_can_power_up(struct net_device *dev)
> if (time_after(jiffies, time_out))
> return -ETIMEDOUT;
>
> - return c_can_start(dev);
> + ret = c_can_start(dev);
> + if (!ret)
> + c_can_enable_all_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> +
> + return ret;
> }
> EXPORT_SYMBOL_GPL(c_can_power_up);
> #endif
> --
> 1.9.0.279.gdc9e3eb
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists