public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Lucas Stach <dev@lynxeye.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 3/8] tegra: usb: fold initial pll setup into board_usb_init
Date: Tue, 30 Oct 2012 14:32:49 +0100	[thread overview]
Message-ID: <1351603969.1434.32.camel@tellur> (raw)
In-Reply-To: <CAPnjgZ18td7PZy4ZbOq5D2XaXBucg3NU8mVYdjXrfvXCLyFz0A@mail.gmail.com>

Am Dienstag, den 30.10.2012, 06:23 -0700 schrieb Simon Glass:
> Hi Lucas,
> 
> On Tue, Oct 30, 2012 at 2:22 AM, Lucas Stach <dev@lynxeye.de> wrote:
> > The setup is trivial, no need to split this out into a separate function.
> >
> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > ---
> >  arch/arm/cpu/armv7/tegra20/usb.c | 15 +++++----------
> >  1 Datei ge?ndert, 5 Zeilen hinzugef?gt(+), 10 Zeilen entfernt(-)
> >
> > diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/arch/arm/cpu/armv7/tegra20/usb.c
> > index 1725cd1..e61bd69 100644
> > --- a/arch/arm/cpu/armv7/tegra20/usb.c
> > +++ b/arch/arm/cpu/armv7/tegra20/usb.c
> > @@ -417,13 +417,6 @@ static int init_ulpi_usb_controller(struct fdt_usb *config)
> >  }
> >  #endif
> >
> > -static void config_clock(const u32 timing[])
> > -{
> > -       clock_start_pll(CLOCK_ID_USB,
> > -               timing[PARAM_DIVM], timing[PARAM_DIVN], timing[PARAM_DIVP],
> > -               timing[PARAM_CPCON], timing[PARAM_LFCON]);
> > -}
> > -
> >  /**
> >   * Add a new USB port to the list of available ports.
> >   *
> > @@ -534,13 +527,15 @@ int board_usb_init(const void *blob)
> >  {
> >         struct fdt_usb config;
> >         unsigned osc_freq = clock_get_rate(CLOCK_ID_OSC);
> > -       enum clock_osc_freq freq;
> >         int node_list[USB_PORTS_MAX];
> >         int node, count, i;
> > +       u32 *timing;
> >
> >         /* Set up the USB clocks correctly based on our oscillator frequency */
> > -       freq = clock_get_osc_freq();
> > -       config_clock(usb_pll[freq]);
> > +       timing = usb_pll[clock_get_osc_freq()];
> > +       clock_start_pll(CLOCK_ID_USB,
> > +               timing[PARAM_DIVM], timing[PARAM_DIVN], timing[PARAM_DIVP],
> > +               timing[PARAM_CPCON], timing[PARAM_LFCON]);
> 
> Sorry I don't see the benefit of this change. The function is there to
> handle a clearly-defined task, hiding the detail of clock config
> elsewhere. It has no effect on code generated.
> 
It's more of a personal thing, that every time there is a function call
it breaks the flow when reading the code. And IMHO it's not worth the
break if the called function does nothing other than just calling
another function.

If other people also dislike the change I may just drop it, but I would
like to hear some more opinions about this first.

Regards,
Lucas

  reply	other threads:[~2012-10-30 13:32 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
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 [this message]
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=1351603969.1434.32.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