| Message ID | 1294752085-30151-2-git-send-email-mkl@pengutronix.de |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
On 01/11/2011 02:21 PM, Marc Kleine-Budde wrote: > This patch cleans up the usage of two macros which specify the mailbox > usage. AT91_MB_RX_FIRST and AT91_MB_RX_NUM define the first and the > number of RX mailboxes. The current driver uses these variables in an > unclean way; assuming that AT91_MB_RX_FIRST is 0; > > This patch cleans up the usage of these macros, no longer assuming > AT91_MB_RX_FIRST == 0. > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> Any comments on this? Marc
On 01/17/2011 08:50 PM, Marc Kleine-Budde wrote: > On 01/11/2011 02:21 PM, Marc Kleine-Budde wrote: >> This patch cleans up the usage of two macros which specify the mailbox >> usage. AT91_MB_RX_FIRST and AT91_MB_RX_NUM define the first and the >> number of RX mailboxes. The current driver uses these variables in an >> unclean way; assuming that AT91_MB_RX_FIRST is 0; >> >> This patch cleans up the usage of these macros, no longer assuming >> AT91_MB_RX_FIRST == 0. >> >> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > > Any comments on this? Looks fine too me. You can add my: Acked-by: Wolfgang Grandegger <wg@grandegger.com> Thanks, Wolfgang. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Marc Kleine-Budde <mkl@pengutronix.de> Date: Mon, 17 Jan 2011 20:50:36 +0100 > On 01/11/2011 02:21 PM, Marc Kleine-Budde wrote: >> This patch cleans up the usage of two macros which specify the mailbox >> usage. AT91_MB_RX_FIRST and AT91_MB_RX_NUM define the first and the >> number of RX mailboxes. The current driver uses these variables in an >> unclean way; assuming that AT91_MB_RX_FIRST is 0; >> >> This patch cleans up the usage of these macros, no longer assuming >> AT91_MB_RX_FIRST == 0. >> >> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > > Any comments on this? I would also seriously like to see these changes get some feedback, they've been rotting in patchwork for more than a week. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/22/2011 01:54 AM, David Miller wrote: > From: Marc Kleine-Budde <mkl@pengutronix.de> > Date: Mon, 17 Jan 2011 20:50:36 +0100 > >> On 01/11/2011 02:21 PM, Marc Kleine-Budde wrote: >>> This patch cleans up the usage of two macros which specify the mailbox >>> usage. AT91_MB_RX_FIRST and AT91_MB_RX_NUM define the first and the >>> number of RX mailboxes. The current driver uses these variables in an >>> unclean way; assuming that AT91_MB_RX_FIRST is 0; >>> >>> This patch cleans up the usage of these macros, no longer assuming >>> AT91_MB_RX_FIRST == 0. >>> >>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> >> >> Any comments on this? > > I would also seriously like to see these changes get some feedback, > they've been rotting in patchwork for more than a week. V2 was OK and it got my "Acked-by" here: http://marc.info/?l=linux-netdev&m=129534267002747&w=2 Wolfgang. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 21, 2011 at 04:54:09PM -0800, David Miller wrote: > > On 01/11/2011 02:21 PM, Marc Kleine-Budde wrote: > >> This patch cleans up the usage of two macros which specify the mailbox > >> usage. AT91_MB_RX_FIRST and AT91_MB_RX_NUM define the first and the > >> number of RX mailboxes. The current driver uses these variables in an > >> unclean way; assuming that AT91_MB_RX_FIRST is 0; > >> > >> This patch cleans up the usage of these macros, no longer assuming > >> AT91_MB_RX_FIRST == 0. > >> > >> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > > > > Any comments on this? > > I would also seriously like to see these changes get some feedback, > they've been rotting in patchwork for more than a week. I have no experience with this specific chip. IMO, this chip errata (as explained in the post) got an elegant solution. That part definitely gets my Acked-by: Kurt Van Dijck <kurt.van.dijck@eia.be> Regards, Kurt -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/22/2011 01:54 AM, David Miller wrote: > From: Marc Kleine-Budde <mkl@pengutronix.de> > Date: Mon, 17 Jan 2011 20:50:36 +0100 > >> On 01/11/2011 02:21 PM, Marc Kleine-Budde wrote: >>> This patch cleans up the usage of two macros which specify the mailbox >>> usage. AT91_MB_RX_FIRST and AT91_MB_RX_NUM define the first and the >>> number of RX mailboxes. The current driver uses these variables in an >>> unclean way; assuming that AT91_MB_RX_FIRST is 0; >>> >>> This patch cleans up the usage of these macros, no longer assuming >>> AT91_MB_RX_FIRST == 0. >>> >>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> >> >> Any comments on this? > > I would also seriously like to see these changes get some feedback, > they've been rotting in patchwork for more than a week. I've just talked to our customer (the one who noticed the problem in the first place). They have the patched driver running over the weekend without problems. I'm going to resend the series the the Acks. regards, Marc
diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c index 7ef83d0..892c3d8 100644 --- a/drivers/net/can/at91_can.c +++ b/drivers/net/can/at91_can.c @@ -2,7 +2,7 @@ * at91_can.c - CAN network driver for AT91 SoC CAN controller * * (C) 2007 by Hans J. Koch <hjk@hansjkoch.de> - * (C) 2008, 2009, 2010 by Marc Kleine-Budde <kernel@pengutronix.de> + * (C) 2008, 2009, 2010, 2011 by Marc Kleine-Budde <kernel@pengutronix.de> * * This software may be distributed under the terms of the GNU General * Public License ("GPL") version 2 as distributed in the 'COPYING' @@ -55,7 +55,8 @@ #define AT91_MB_RX_MASK(i) ((1 << (i)) - 1) #define AT91_MB_RX_SPLIT 8 #define AT91_MB_RX_LOW_LAST (AT91_MB_RX_SPLIT - 1) -#define AT91_MB_RX_LOW_MASK (AT91_MB_RX_MASK(AT91_MB_RX_SPLIT)) +#define AT91_MB_RX_LOW_MASK (AT91_MB_RX_MASK(AT91_MB_RX_SPLIT) & \ + ~AT91_MB_RX_MASK(AT91_MB_RX_FIRST)) #define AT91_MB_TX_NUM (1 << AT91_MB_TX_SHIFT) #define AT91_MB_TX_FIRST (AT91_MB_RX_LAST + 1) @@ -254,7 +255,8 @@ static void at91_setup_mailboxes(struct net_device *dev) set_mb_mode_prio(priv, i, AT91_MB_MODE_TX, 0); /* Reset tx and rx helper pointers */ - priv->tx_next = priv->tx_echo = priv->rx_next = 0; + priv->tx_next = priv->tx_echo = 0; + priv->rx_next = AT91_MB_RX_FIRST; } static int at91_set_bittiming(struct net_device *dev) @@ -590,10 +592,10 @@ static int at91_poll_rx(struct net_device *dev, int quota) "order of incoming frames cannot be guaranteed\n"); again: - for (mb = find_next_bit(addr, AT91_MB_RX_NUM, priv->rx_next); - mb < AT91_MB_RX_NUM && quota > 0; + for (mb = find_next_bit(addr, AT91_MB_RX_LAST + 1, priv->rx_next); + mb < AT91_MB_RX_LAST + 1 && quota > 0; reg_sr = at91_read(priv, AT91_SR), - mb = find_next_bit(addr, AT91_MB_RX_NUM, ++priv->rx_next)) { + mb = find_next_bit(addr, AT91_MB_RX_LAST + 1, ++priv->rx_next)) { at91_read_msg(dev, mb); /* reactivate mailboxes */ @@ -610,8 +612,8 @@ static int at91_poll_rx(struct net_device *dev, int quota) /* upper group completed, look again in lower */ if (priv->rx_next > AT91_MB_RX_LOW_LAST && - quota > 0 && mb >= AT91_MB_RX_NUM) { - priv->rx_next = 0; + quota > 0 && mb > AT91_MB_RX_LAST) { + priv->rx_next = AT91_MB_RX_FIRST; goto again; }
This patch cleans up the usage of two macros which specify the mailbox usage. AT91_MB_RX_FIRST and AT91_MB_RX_NUM define the first and the number of RX mailboxes. The current driver uses these variables in an unclean way; assuming that AT91_MB_RX_FIRST is 0; This patch cleans up the usage of these macros, no longer assuming AT91_MB_RX_FIRST == 0. Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> --- drivers/net/can/at91_can.c | 18 ++++++++++-------- 1 files changed, 10 insertions(+), 8 deletions(-)