From: Lucas Stach <dev@lynxeye.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/8] tegra: usb: make controller init functions more self contained
Date: Tue, 30 Oct 2012 14:16:43 +0100 [thread overview]
Message-ID: <1351603003.1434.28.camel@tellur> (raw)
In-Reply-To: <CAPnjgZ1sYZC6VVJRehm=wfR5sTOcwOST2udJcoMWSzPSy1ovKw@mail.gmail.com>
Hello Simon,
Am Dienstag, den 30.10.2012, 06:03 -0700 schrieb Simon Glass:
> Hi Lucas,
>
> On Tue, Oct 30, 2012 at 2:22 AM, Lucas Stach <dev@lynxeye.de> wrote:
> > There is no need to pass around all those parameters. The init functions
> > are able to easily extract all the needed setup info on their own.
> >
> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > ---
> > arch/arm/cpu/armv7/tegra20/usb.c | 24 ++++++++++++------------
> > 1 Datei ge?ndert, 12 Zeilen hinzugef?gt(+), 12 Zeilen entfernt(-)
>
> I'm not sure I agree with the premise of this patch.
>
> At the top level it calls clock_get_osc_freq() to get the frequency.
> That is then passed to the two places that need it.
>
> It doesn't seem right to me to call clock_get_osc_freq() again in the
> lower level function just to avoid a parameter. On ARM at least a few
> parameters are a cheap way of passing data around.
>
The intent of this patch is not really to save up on parameters passed,
but to make it possible to later move out the controller initialization
into the ehci_hcd_init function without having to save away this global
state for later use.
We have to init at most 2 controllers where timing matters, so I think
it's the right thing to get the SoC global clock state at those two
occasions to avoid inflating the file global state.
> It also allows the lower-level functions to deal with what they need
> to, rather than all functions having to reference the global state
> independently, each one digging down to what it actually needs.
>
The controller init functions get passed the state only of the one port
they have to initialize. There is no point in extracting things at an
upper level and passing it into the functions, if it's exactly the same
thing that is stored in the port state.
Regards,
Lucas
next prev parent reply other threads:[~2012-10-30 13:16 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-30 9:22 [U-Boot] [PATCH 1/8] tegra: usb: convert USB_PORTS_MAX to be a define Lucas Stach
2012-10-30 9:22 ` [U-Boot] [PATCH 2/8] tegra: usb: make controller init functions more self contained Lucas Stach
2012-10-30 13:03 ` Simon Glass
2012-10-30 13:16 ` Lucas Stach [this message]
2012-10-30 15:35 ` Simon Glass
2012-10-30 9:22 ` [U-Boot] [PATCH 3/8] tegra: usb: fold initial pll setup into board_usb_init Lucas Stach
2012-10-30 13:23 ` Simon Glass
2012-10-30 13:32 ` Lucas Stach
2012-10-30 9:22 ` [U-Boot] [PATCH 4/8] tegra: usb: remove unneeded function parameter Lucas Stach
2012-10-30 13:18 ` Simon Glass
2012-10-30 9:22 ` [U-Boot] [PATCH 5/8] tegra: usb: move controller init into start_port Lucas Stach
2012-10-30 10:59 ` Marek Vasut
2012-10-30 12:12 ` Lucas Stach
2012-10-30 12:33 ` Marek Vasut
2012-10-30 12:44 ` Lucas Stach
2012-10-30 18:34 ` Stephen Warren
2012-10-30 13:27 ` Simon Glass
2012-10-30 13:37 ` Lucas Stach
2012-10-30 13:48 ` Simon Glass
2012-10-30 13:54 ` Lucas Stach
2012-10-30 14:03 ` Simon Glass
2012-10-30 9:22 ` [U-Boot] [PATCH 6/8] tegra: usb: various small cleanups Lucas Stach
2012-10-30 13:31 ` Simon Glass
2012-10-30 9:22 ` [U-Boot] [PATCH 7/8] tegra: usb: move implementation into right directory Lucas Stach
2012-10-30 13:33 ` Simon Glass
2012-10-30 13:38 ` Lucas Stach
2012-10-30 13:53 ` Simon Glass
2012-10-30 14:03 ` Lucas Stach
2012-10-30 16:11 ` Tom Warren
2012-10-30 16:18 ` Simon Glass
2012-10-30 18:38 ` Stephen Warren
2012-10-30 18:45 ` Lucas Stach
2012-10-30 18:51 ` Stephen Warren
2012-10-30 18:58 ` Lucas Stach
2012-10-30 9:22 ` [U-Boot] [PATCH 8/8] tegra: usb: move [start|stop]_port into ehci_hcd_[init|stop] Lucas Stach
2012-10-30 13:39 ` Simon Glass
2012-10-30 13:09 ` [U-Boot] [PATCH 1/8] tegra: usb: convert USB_PORTS_MAX to be a define Simon Glass
2012-10-30 13:11 ` Marek Vasut
2012-10-30 13:40 ` Simon Glass
2012-11-02 20:41 ` 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=1351603003.1434.28.camel@tellur \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox