All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfgang Grandegger <wg@grandegger.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/3] smsc95xx: in smsc95xx_set_multicast write to reg
Date: Mon, 14 Nov 2011 13:18:18 +0100	[thread overview]
Message-ID: <4EC1070A.1070801@grandegger.com> (raw)
In-Reply-To: <CAPnjgZ2pU+_wFSnX0=RgXwF5vGgwzSRqhd=D1vLiNbJ62bwbrw@mail.gmail.com>

Hi Simon,

On 11/11/2011 04:35 PM, Simon Glass wrote:
> Hi Wolfgang,
> 
> On Fri, Nov 11, 2011 at 2:59 AM, Wolfgang Grandegger <wg@denx.de> wrote:
>> The write to the mac_cr register was missing. This usually not
>> cause an issue before, since the next function writing the
>> register's shadow copy into the register would do it as a side
>> effect.
>>
>> Signed-off-by: Wolfgang Grandegger <wg@denx.de>
>> Cc: Simon Glass <sjg@chromium.org>
>> ---
>>  drivers/usb/eth/smsc95xx.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/usb/eth/smsc95xx.c b/drivers/usb/eth/smsc95xx.c
>> index eb529f1..16e24bd 100644
>> --- a/drivers/usb/eth/smsc95xx.c
>> +++ b/drivers/usb/eth/smsc95xx.c
>> @@ -428,6 +428,8 @@ static void smsc95xx_set_multicast(struct ueth_data *dev)
>>  {
>>        /* No multicast in u-boot */
>>        dev->mac_cr &= ~(MAC_CR_PRMS_ | MAC_CR_MCPAS_ | MAC_CR_HPFILT_);
>> +
>> +       smsc95xx_write_reg(dev, MAC_CR, dev->mac_cr);
> 
> It seems a bit odd - what problem does your addition actually fix? At
> present both smsc95xx_start_tx_path() and smsc95xx_start_rx_path()
> write to this register, so it will be written three times in quick
> succession. If there are no other callers to smsc95xx_set_multicast()
> outside init, then perhaps we should just write it once in init, after
> calling the three functions?

The name "set_multicast" associated to me that the relevant register is
really set. But from the functional poit of few, it is not necessary.
Well, it's a minor issue and maybe not worth fixing. So, let's drop this
patch.

Wolfgang.

  reply	other threads:[~2011-11-14 12:18 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-11 10:59 [U-Boot] [PATCH 0/3] smsc95xx: Fix MAC address programming and some minor issues Wolfgang Grandegger
2011-11-11 10:59 ` [U-Boot] [PATCH 1/3] smsc95xx: Fix MAC address programming Wolfgang Grandegger
2011-11-11 11:04   ` Marek Vasut
2011-11-11 12:33     ` Wolfgang Grandegger
2011-11-11 15:18       ` Mike Frysinger
2011-11-14  8:25         ` Wolfgang Grandegger
2011-11-14 16:50           ` Mike Frysinger
2011-11-15  9:25             ` Wolfgang Grandegger
2011-11-11 15:20   ` Mike Frysinger
2011-11-11 15:22   ` Mike Frysinger
2011-11-11 10:59 ` [U-Boot] [PATCH 2/3] smsc95xx: in smsc95xx_set_multicast write to reg Wolfgang Grandegger
2011-11-11 15:35   ` Simon Glass
2011-11-14 12:18     ` Wolfgang Grandegger [this message]
2011-11-14 20:06       ` Simon Glass
2011-11-11 10:59 ` [U-Boot] [PATCH 3/3] smsc95xx: remove an unecessary debug messages Wolfgang Grandegger
2011-11-11 15:26   ` Simon Glass
2011-11-14  8:33     ` Wolfgang Grandegger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EC1070A.1070801@grandegger.com \
    --to=wg@grandegger.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.