[U-Boot] [PATCH 2/3] smsc95xx: in smsc95xx_set_multicast write to reg

Wolfgang Grandegger wg at grandegger.com
Mon Nov 14 13:18:18 CET 2011


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 at 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 at denx.de>
>> Cc: Simon Glass <sjg at 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.


More information about the U-Boot mailing list