public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] Move ICS CLK chip frequenty calculation code into a common board library
Date: Sat, 22 May 2010 00:35:29 +0200	[thread overview]
Message-ID: <20100521223529.94F83CCF026@gemini.denx.de> (raw)
In-Reply-To: <7E86BC8F-7E3F-49B1-B1C5-B21991303AF5@kernel.crashing.org>

Dear Kumar Gala,

In message <7E86BC8F-7E3F-49B1-B1C5-B21991303AF5@kernel.crashing.org> you wrote:
> 
> <galak@kernel.crashing.org> wrote:
> > 
> >> +#ifdef CONFIG_FSL_NGPIXIS
> >> +#include "ngpixis.h"
> >> +#define PIXIS_VSYSCLK0         offsetof(struct ngpixis, sclk[0])
> >> +#define PIXIS_VSYSCLK1         offsetof(struct ngpixis, sclk[1])
> >> +#define PIXIS_VSYSCLK2         offsetof(struct ngpixis, sclk[2])
> >> +#define PIXIS_VDDRCLK0         offsetof(struct ngpixis, dclk[0])
> >> +#define PIXIS_VDDRCLK1         offsetof(struct ngpixis, dclk[1])
> >> +#define PIXIS_VDDRCLK2         offsetof(struct ngpixis, dclk[2])
> >> +#endif
> > 
> > Yuck.  I specifically created the ngpixis_t structure because Wolfgang
> > objected to the use of macros for offsets.
> > 
> > But I understand why you did it.  It's the easiest way to maintain
> > compatibility between pixis and ngpixis.
>
> Wolfgang, is this an issue?

This, in itself, it not an issue. 

The issue comes when you wrote code like this:

+	return ics307_clk_freq (
+			in_8(pixis_base + PIXIS_VSYSCLK0),
+			in_8(pixis_base + PIXIS_VSYSCLK1),
+			in_8(pixis_base + PIXIS_VSYSCLK2)
+			);
+}
+
+unsigned long get_board_ddr_clk(void)
+{
+	u8 *pixis_base = (u8 *)PIXIS_BASE;
+
+	return ics307_clk_freq (
+			in_8(pixis_base + PIXIS_VDDRCLK0),
+			in_8(pixis_base + PIXIS_VDDRCLK1),
+			in_8(pixis_base + PIXIS_VDDRCLK2)
+			);

This clearly gets a NAK because you should use a struct, and not base
address + offset notation.

> The other option is have 3 different structs:
>
> On p2020: #define PIXIS_VSYSCLK0         offsetof(struct ngpixis, > sclk[0])
> on  8572: #define PIXIS_VSYSCLK0	0x1C
> on  8536: #define PIXIS_VSYSCLK0	0x1A

I see #defines here, no structs.

I'm not sure if you need different structs (haven't looked into
these), or if clever notation of the struct is sufficient.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A committee is a life form with six or more legs and no brain.
                              -- Lazarus Long, "Time Enough For Love"

  reply	other threads:[~2010-05-21 22:35 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-21  9:18 [U-Boot] [PATCH] Move ICS CLK chip frequenty calculation code into a common board library Kumar Gala
2010-05-21  9:18 ` [U-Boot] [PATCH] ppc/85xx: Convert MPC8572DS to using board common ICS307 code Kumar Gala
2010-05-21  9:18   ` [U-Boot] [PATCH] ppc/85xx: Convert MPC8536DS " Kumar Gala
2010-05-21 15:57 ` [U-Boot] [PATCH] Move ICS CLK chip frequenty calculation code into a common board library Timur Tabi
2010-05-21 22:19   ` Kumar Gala
2010-05-21 22:29     ` Wolfgang Denk
2010-05-21 22:22   ` Kumar Gala
2010-05-21 22:35     ` Wolfgang Denk [this message]
2010-05-21 18:42 ` Timur Tabi
2010-05-22 22:41   ` Kumar Gala
2010-05-21 18:59 ` Timur Tabi
2010-05-21 19:35   ` Timur Tabi
2010-05-21 19:52     ` Wolfgang Denk
2010-05-21 19:58       ` Timur Tabi
2010-05-21 20:16         ` Wolfgang Denk
2010-05-21 20:28           ` Timur Tabi
2010-05-21 21:03             ` Wolfgang Denk
2010-05-21 21:09               ` Timur Tabi
2010-05-22 22:39           ` Kumar Gala
2010-05-22 22:36 ` [U-Boot] [PATCH v2] " Kumar Gala
2010-05-24 14:52   ` Timur Tabi
2010-05-24 18:57     ` Kumar Gala
2010-05-24 19:06       ` Timur Tabi
2010-05-24 20:09 ` [U-Boot] [PATCH v3] " Kumar Gala
2010-05-24 20:12   ` Timur Tabi
2010-06-30  9:52   ` Kumar Gala
2010-07-14 19:33     ` Wolfgang Denk
2010-07-14 21:21     ` [U-Boot] [PATCH v4] Move ICS CLK chip frequency " Kumar Gala

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=20100521223529.94F83CCF026@gemini.denx.de \
    --to=wd@denx.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