All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lucas Stach <dev@lynxeye.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 3/6] net: asix: split out basic reset function
Date: Thu, 23 Aug 2012 10:37:29 +0200	[thread overview]
Message-ID: <1345711049.2596.15.camel@selen> (raw)
In-Reply-To: <201208222301.05621.vapier@gentoo.org>

Hi Mike,

Am Mittwoch, den 22.08.2012, 23:01 -0400 schrieb Mike Frysinger:
> On Wednesday 22 August 2012 06:09:04 Lucas Stach wrote:
> > The basic device reset ensures that the device is ready to
> > service commands and does not need to get redone before each
> > network operation.
> > 
> > Split out the basic reset from asix_init() and instead call it
> > from asix_eth_get_info(), so that it only gets called once.
> 
> i'm afraid this is wrong.  the register step (which asix_eth_get_info is 
> afaict) should not touch the hardware (other than to extract the MAC address).  
> the init func is supposed to bring up the hardware from scratch since the 
> expectation is that the halt func brings it down completely.  if it's not 
> really possible to completely "bring down" the hardware, then have the 
> asix_init func declare a local static int so that it does the "full" bring up 
> only once.
> 
> so NAK this patch as is
> -mike

I still think the patch does the right thing. Let's look at the call
chain. When doing a usb start of the controller where the ethernet
device is attached we do the following to register the device:
1. probe (just check if we found suitable hardware, don't touch it yet)
2. get_info (at this point we extract the MAC, which means we have to
bring up the hardware at a basic level, to even be able to touch the
registers)
3. write_hwaddr (eth core sets the MAC address of the device, remember
this is only done _once_ for each device and also needs basic init)

Every network operation basically does first a init() and then a halt().
If we would bring down the device here to a point where it's completely
reset, we would also lose the MAC address set in the register step. This
is clearly not what we want, but also we don't want to set the MAC over
and over again with each network operation. So IMHO halt() should only
bring down the device to a state where the current ethernet connection
is gone. Therefore init() only needs to bring up the ethernet connection
from this state. The basic device initialisation (including the MAC)
should be persistent across multiple network operations.

This is the behaviour this patch implements, aside from halt() not
really doing it's job in the asix driver, but that is for another patch.

Thanks,
Lucas

  reply	other threads:[~2012-08-23  8:37 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-22 10:09 [U-Boot] [PATCH 0/6] net: ASIX AX88772B enablement Lucas Stach
2012-08-22 10:09 ` [U-Boot] [PATCH 1/6] net: introduce transparent driver private in ueth_data Lucas Stach
2012-08-22 18:21   ` Marek Vasut
2012-08-22 18:49   ` Joe Hershberger
2012-08-22 10:09 ` [U-Boot] [PATCH 2/6] net: usbeth: remove misleading warning Lucas Stach
2012-08-22 18:22   ` Marek Vasut
2012-08-22 18:32     ` Lucas Stach
2012-08-22 18:46       ` Joe Hershberger
2012-08-22 10:09 ` [U-Boot] [PATCH 3/6] net: asix: split out basic reset function Lucas Stach
2012-08-22 18:23   ` Marek Vasut
2012-08-22 18:52   ` Joe Hershberger
2012-08-23  3:01   ` Mike Frysinger
2012-08-23  8:37     ` Lucas Stach [this message]
2012-08-22 10:09 ` [U-Boot] [PATCH 4/6] net: asix: add write_hwaddr function Lucas Stach
2012-08-22 18:25   ` Marek Vasut
2012-08-22 19:04   ` Joe Hershberger
2012-08-22 10:09 ` [U-Boot] [PATCH 5/6] net: asix: add read_mac function Lucas Stach
2012-08-22 18:26   ` Marek Vasut
2012-08-22 18:59   ` Joe Hershberger
2012-08-22 10:09 ` [U-Boot] [PATCH 6/6] net: asix: add AX88772B support Lucas Stach
2012-08-22 18:26   ` Marek Vasut
2012-08-22 19:01   ` Joe Hershberger
2012-08-22 18:20 ` [U-Boot] [PATCH 0/6] net: ASIX AX88772B enablement Marek Vasut

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=1345711049.2596.15.camel@selen \
    --to=dev@lynxeye.de \
    --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.