Re: [PATCH 3/9] 8xx: Add pin and clock setting functions.
On Fri, 31 Aug 2007 15:44:18 -0500 Scott Wood wrote: + u32 mask = 7; + gotta at least briefly explain the clue here, too. I'm not sure what you mean... what exactly are you asking me to explain? Note that this code is mostly duplicated from the existing CPM2 version. Then it would be good to mention that in short comment block before function. We're adding helper functions and should be ready that something somewhere won't work as expected. Could you elaborate on what you expect to possibly not work, or what we should do to be ready? Some new PQ-like board being added to powerpc. I mean, each newly-added non-static function should have some sort of description unless it(function) is completely self-explanatory. Just a few words either here as a comment or in patch description, what the function supposed to do and which way, so that someone not familiar with cpm/cpm2, would quickly understand what's happening in there. -- Sincerely, Vitaly ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 3/9] 8xx: Add pin and clock setting functions.
On Wed, Sep 05, 2007 at 11:39:57AM +0400, Vitaly Bordug wrote: Note that this code is mostly duplicated from the existing CPM2 version. Then it would be good to mention that in short comment block before function. The code would very quickly become unreadable if we kept comments around for every movement of code. Given the current state of things, it's pretty much a given that most functions in commproc.c have a corresponding function in cpm2_common.c, and vice versa. I'd like to merge some of it at some point. We're adding helper functions and should be ready that something somewhere won't work as expected. Could you elaborate on what you expect to possibly not work, or what we should do to be ready? Some new PQ-like board being added to powerpc. I mean, each newly-added non-static function should have some sort of description unless it(function) is completely self-explanatory. I thought it was completely self-explanatory. Just a few words either here as a comment or in patch description, what the function supposed to do and which way, so that someone not familiar with cpm/cpm2, would quickly understand what's happening in there. If they're not familiar with CPM, and want to understand how hardware setup for it works, the user's manual for one of the chips would be a good place to start... As it is, it's pretty clear without knowledge of the CPM that it's a helper function that sets and clears bits in certain registers. Since we seem to have differing views of the target audience, could you suggest a specific wording that you're looking for? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 3/9] 8xx: Add pin and clock setting functions.
These let board code set up pins and clocks without having to
put magic numbers directly into the registers.
The clock function is mostly duplicated from the cpm2 version;
hopefully this stuff can be merged at some point.
Signed-off-by: Scott Wood [EMAIL PROTECTED]
---
arch/powerpc/sysdev/commproc.c | 201
include/asm-powerpc/commproc.h | 49 ++
2 files changed, 250 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/sysdev/commproc.c b/arch/powerpc/sysdev/commproc.c
index af26659..7497745 100644
--- a/arch/powerpc/sysdev/commproc.c
+++ b/arch/powerpc/sysdev/commproc.c
@@ -405,3 +405,204 @@ uint cpm_dpram_phys(u8 *addr)
return (dpram_pbase + (uint)(addr - (u8 __force *)dpram_vbase));
}
EXPORT_SYMBOL(cpm_dpram_addr);
+
+struct cpm_ioport16 {
+ __be16 dir, par, sor, dat, intr;
+ __be16 res[3];
+};
+
+struct cpm_ioport32 {
+ __be32 dir, par, sor;
+};
+
+static void cpm1_set_pin32(int port, int pin, int flags)
+{
+ struct cpm_ioport32 __iomem *iop;
+ pin = 1 (31 - pin);
+
+ if (port == CPM_PORTB)
+ iop = (struct cpm_ioport32 __iomem *)
+ mpc8xx_immr-im_cpm.cp_pbdir;
+ else
+ iop = (struct cpm_ioport32 __iomem *)
+ mpc8xx_immr-im_cpm.cp_pedir;
+
+ if (flags CPM_PIN_OUTPUT)
+ setbits32(iop-dir, pin);
+ else
+ clrbits32(iop-dir, pin);
+
+ if (!(flags CPM_PIN_GPIO))
+ setbits32(iop-par, pin);
+ else
+ clrbits32(iop-par, pin);
+
+ if (port == CPM_PORTE) {
+ if (flags CPM_PIN_SECONDARY)
+ setbits32(iop-sor, pin);
+ else
+ clrbits32(iop-sor, pin);
+
+ if (flags CPM_PIN_OPENDRAIN)
+ setbits32(mpc8xx_immr-im_cpm.cp_peodr, pin);
+ else
+ clrbits32(mpc8xx_immr-im_cpm.cp_peodr, pin);
+ }
+}
+
+static void cpm1_set_pin16(int port, int pin, int flags)
+{
+ struct cpm_ioport16 __iomem *iop =
+ (struct cpm_ioport16 __iomem *)mpc8xx_immr-im_ioport;
+
+ pin = 1 (15 - pin);
+
+ if (port != 0)
+ iop += port - 1;
+
+ if (flags CPM_PIN_OUTPUT)
+ setbits16(iop-dir, pin);
+ else
+ clrbits16(iop-dir, pin);
+
+ if (!(flags CPM_PIN_GPIO))
+ setbits16(iop-par, pin);
+ else
+ clrbits16(iop-par, pin);
+
+ if (port == CPM_PORTC) {
+ if (flags CPM_PIN_SECONDARY)
+ setbits16(iop-sor, pin);
+ else
+ clrbits16(iop-sor, pin);
+ }
+}
+
+void cpm1_set_pin(enum cpm_port port, int pin, int flags)
+{
+ if (port == CPM_PORTB || port == CPM_PORTE)
+ cpm1_set_pin32(port, pin, flags);
+ else
+ cpm1_set_pin16(port, pin, flags);
+}
+
+int cpm1_clk_setup(enum cpm_clk_target target, int clock, int mode)
+{
+ int shift;
+ int i, bits = 0;
+ u32 __iomem *reg;
+ u32 mask = 7;
+
+ u8 clk_map[][3] = {
+ {CPM_CLK_SCC1, CPM_BRG1, 0},
+ {CPM_CLK_SCC1, CPM_BRG2, 1},
+ {CPM_CLK_SCC1, CPM_BRG3, 2},
+ {CPM_CLK_SCC1, CPM_BRG4, 3},
+ {CPM_CLK_SCC1, CPM_CLK1, 4},
+ {CPM_CLK_SCC1, CPM_CLK2, 5},
+ {CPM_CLK_SCC1, CPM_CLK3, 6},
+ {CPM_CLK_SCC1, CPM_CLK4, 7},
+
+ {CPM_CLK_SCC2, CPM_BRG1, 0},
+ {CPM_CLK_SCC2, CPM_BRG2, 1},
+ {CPM_CLK_SCC2, CPM_BRG3, 2},
+ {CPM_CLK_SCC2, CPM_BRG4, 3},
+ {CPM_CLK_SCC2, CPM_CLK1, 4},
+ {CPM_CLK_SCC2, CPM_CLK2, 5},
+ {CPM_CLK_SCC2, CPM_CLK3, 6},
+ {CPM_CLK_SCC2, CPM_CLK4, 7},
+
+ {CPM_CLK_SCC3, CPM_BRG1, 0},
+ {CPM_CLK_SCC3, CPM_BRG2, 1},
+ {CPM_CLK_SCC3, CPM_BRG3, 2},
+ {CPM_CLK_SCC3, CPM_BRG4, 3},
+ {CPM_CLK_SCC3, CPM_CLK5, 4},
+ {CPM_CLK_SCC3, CPM_CLK6, 5},
+ {CPM_CLK_SCC3, CPM_CLK7, 6},
+ {CPM_CLK_SCC3, CPM_CLK8, 7},
+
+ {CPM_CLK_SCC4, CPM_BRG1, 0},
+ {CPM_CLK_SCC4, CPM_BRG2, 1},
+ {CPM_CLK_SCC4, CPM_BRG3, 2},
+ {CPM_CLK_SCC4, CPM_BRG4, 3},
+ {CPM_CLK_SCC4, CPM_CLK5, 4},
+ {CPM_CLK_SCC4, CPM_CLK6, 5},
+ {CPM_CLK_SCC4, CPM_CLK7, 6},
+ {CPM_CLK_SCC4, CPM_CLK8, 7},
+
+ {CPM_CLK_SMC1, CPM_BRG1, 0},
+ {CPM_CLK_SMC1, CPM_BRG2, 1},
+ {CPM_CLK_SMC1, CPM_BRG3, 2},
+ {CPM_CLK_SMC1, CPM_BRG4, 3},
+ {CPM_CLK_SMC1, CPM_CLK1, 4},
+ {CPM_CLK_SMC1, CPM_CLK2, 5},
+ {CPM_CLK_SMC1, CPM_CLK3, 6},
+ {CPM_CLK_SMC1, CPM_CLK4,
Re: [PATCH 3/9] 8xx: Add pin and clock setting functions.
On Thu, Aug 30, 2007 at 01:38:33AM +0400, Vitaly Bordug wrote:
On Tue, 28 Aug 2007 15:17:19 -0500
Scott Wood wrote:
+struct cpm_ioport16 {
+ __be16 dir, par, sor, dat, intr;
+ __be16 res[3];
+};
+
Hmm. If we are using such a non-standard types, it worths at least
explanation why...
It's so I can index into the port array. The standard struct has
non-indexable names such as padir, pcpar, etc.
+struct cpm_ioport32 {
+ __be32 dir, par, sor;
+};
+
+static void cpm1_set_pin32(int port, int pin, int flags)
+{
+ struct cpm_ioport32 __iomem *iop;
+ pin = 1 (31 - pin);
+
+ if (port == 1)
Probably put here define or alike so that we wouldn't have confusion what
1/whatever port number does mean.
Or some comment explaining for PQ newcomer what's going on here. ditto below.
I suppose I could add CPM_PORTA, CPM_PORTB, etc. defines.
+int cpm1_clk_setup(enum cpm_clk_target target, int clock, int mode)
+{
+ int shift;
+ int i, bits = 0;
+ u32 __iomem *reg;
+ u32 mask = 7;
+
gotta at least briefly explain the clue here, too.
I'm not sure what you mean... what exactly are you asking me to explain?
Note that this code is mostly duplicated from the existing CPM2 version.
We're adding helper functions and should be ready that something
somewhere won't work as expected.
Could you elaborate on what you expect to possibly not work, or what we
should do to be ready?
diff --git a/include/asm-powerpc/commproc.h
b/include/asm-powerpc/commproc.h index ccb32cd..a95a434 100644
--- a/include/asm-powerpc/commproc.h
+++ b/include/asm-powerpc/commproc.h
@@ -692,4 +692,45 @@ extern void cpm_free_handler(int vec);
#define IMAP_ADDR (get_immrbase())
#define IMAP_SIZE ((uint)(64 * 1024))
Pull from the dts?
It is. I kept IMAP_ADDR around to ease interdependencies on changes in
drivers/net/fs_enet. It appears IMAP_SIZE isn't used.
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 3/9] 8xx: Add pin and clock setting functions.
On Tue, 28 Aug 2007 15:17:19 -0500
Scott Wood wrote:
These let board code set up pins and clocks without having to
put magic numbers directly into the registers.
I personally is not fond of such idea, but it would make this more
understandable eases transfer to feature_call
or qe pin setting stuff (though the latter should be reworked at some point too
imho).
Signed-off-by: Scott Wood [EMAIL PROTECTED]
---
arch/powerpc/sysdev/commproc.c | 201
include/asm-powerpc/commproc.h | 41 2 files changed, 242
insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/sysdev/commproc.c
b/arch/powerpc/sysdev/commproc.c index af26659..a21a292 100644
--- a/arch/powerpc/sysdev/commproc.c
+++ b/arch/powerpc/sysdev/commproc.c
@@ -405,3 +405,204 @@ uint cpm_dpram_phys(u8 *addr)
return (dpram_pbase + (uint)(addr - (u8 __force
*)dpram_vbase)); }
EXPORT_SYMBOL(cpm_dpram_addr);
+
+struct cpm_ioport16 {
+ __be16 dir, par, sor, dat, intr;
+ __be16 res[3];
+};
+
Hmm. If we are using such a non-standard types, it worths at least explanation
why...
+struct cpm_ioport32 {
+ __be32 dir, par, sor;
+};
+
+static void cpm1_set_pin32(int port, int pin, int flags)
+{
+ struct cpm_ioport32 __iomem *iop;
+ pin = 1 (31 - pin);
+
+ if (port == 1)
Probably put here define or alike so that we wouldn't have confusion what
1/whatever port number does mean.
Or some comment explaining for PQ newcomer what's going on here. ditto below.
+ iop = (struct cpm_ioport32 __iomem *)
+ mpc8xx_immr-im_cpm.cp_pbdir;
+ else
+ iop = (struct cpm_ioport32 __iomem *)
+ mpc8xx_immr-im_cpm.cp_pedir;
+
+ if (flags CPM_PIN_OUTPUT)
+ setbits32(iop-dir, pin);
+ else
+ clrbits32(iop-dir, pin);
+
+ if (!(flags CPM_PIN_GPIO))
+ setbits32(iop-par, pin);
+ else
+ clrbits32(iop-par, pin);
+
+ if (port == 4) {
+ if (flags CPM_PIN_SECONDARY)
+ setbits32(iop-sor, pin);
+ else
+ clrbits32(iop-sor, pin);
+
+ if (flags CPM_PIN_OPENDRAIN)
+ setbits32(mpc8xx_immr-im_cpm.cp_peodr,
pin);
+ else
+ clrbits32(mpc8xx_immr-im_cpm.cp_peodr,
pin);
+ }
+}
+
+static void cpm1_set_pin16(int port, int pin, int flags)
+{
+ struct cpm_ioport16 __iomem *iop =
+ (struct cpm_ioport16 __iomem
*)mpc8xx_immr-im_ioport; +
+ pin = 1 (15 - pin);
+
+ if (port != 0)
+ iop += port - 1;
+
+ if (flags CPM_PIN_OUTPUT)
+ setbits16(iop-dir, pin);
+ else
+ clrbits16(iop-dir, pin);
+
+ if (!(flags CPM_PIN_GPIO))
+ setbits16(iop-par, pin);
+ else
+ clrbits16(iop-par, pin);
+
+ if (port == 2) {
+ if (flags CPM_PIN_SECONDARY)
+ setbits16(iop-sor, pin);
+ else
+ clrbits16(iop-sor, pin);
+ }
+}
+
+void cpm1_set_pin(int port, int pin, int flags)
+{
+ if (port == 1 || port == 4)
+ cpm1_set_pin32(port, pin, flags);
+ else
+ cpm1_set_pin16(port, pin, flags);
+}
+
+int cpm1_clk_setup(enum cpm_clk_target target, int clock, int mode)
+{
+ int shift;
+ int i, bits = 0;
+ u32 __iomem *reg;
+ u32 mask = 7;
+
gotta at least briefly explain the clue here, too. We're adding helper
functions and should be ready that something somewhere
won't work as expected.
+ u8 clk_map[][3] = {
+ {CPM_CLK_SCC1, CPM_BRG1, 0},
+ {CPM_CLK_SCC1, CPM_BRG2, 1},
+ {CPM_CLK_SCC1, CPM_BRG3, 2},
+ {CPM_CLK_SCC1, CPM_BRG4, 3},
+ {CPM_CLK_SCC1, CPM_CLK1, 4},
+ {CPM_CLK_SCC1, CPM_CLK2, 5},
+ {CPM_CLK_SCC1, CPM_CLK3, 6},
+ {CPM_CLK_SCC1, CPM_CLK4, 7},
+
+ {CPM_CLK_SCC2, CPM_BRG1, 0},
+ {CPM_CLK_SCC2, CPM_BRG2, 1},
+ {CPM_CLK_SCC2, CPM_BRG3, 2},
+ {CPM_CLK_SCC2, CPM_BRG4, 3},
+ {CPM_CLK_SCC2, CPM_CLK1, 4},
+ {CPM_CLK_SCC2, CPM_CLK2, 5},
+ {CPM_CLK_SCC2, CPM_CLK3, 6},
+ {CPM_CLK_SCC2, CPM_CLK4, 7},
+
+ {CPM_CLK_SCC3, CPM_BRG1, 0},
+ {CPM_CLK_SCC3, CPM_BRG2, 1},
+ {CPM_CLK_SCC3, CPM_BRG3, 2},
+ {CPM_CLK_SCC3, CPM_BRG4, 3},
+ {CPM_CLK_SCC3, CPM_CLK5, 4},
+ {CPM_CLK_SCC3, CPM_CLK6, 5},
+ {CPM_CLK_SCC3, CPM_CLK7, 6},
+ {CPM_CLK_SCC3, CPM_CLK8, 7},
+
+ {CPM_CLK_SCC4, CPM_BRG1, 0},
+ {CPM_CLK_SCC4, CPM_BRG2, 1},
+ {CPM_CLK_SCC4, CPM_BRG3, 2},
+ {CPM_CLK_SCC4, CPM_BRG4, 3},
+ {CPM_CLK_SCC4, CPM_CLK5, 4},
+
[PATCH 3/9] 8xx: Add pin and clock setting functions.
These let board code set up pins and clocks without having to
put magic numbers directly into the registers.
Signed-off-by: Scott Wood [EMAIL PROTECTED]
---
arch/powerpc/sysdev/commproc.c | 201
include/asm-powerpc/commproc.h | 41
2 files changed, 242 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/sysdev/commproc.c b/arch/powerpc/sysdev/commproc.c
index af26659..a21a292 100644
--- a/arch/powerpc/sysdev/commproc.c
+++ b/arch/powerpc/sysdev/commproc.c
@@ -405,3 +405,204 @@ uint cpm_dpram_phys(u8 *addr)
return (dpram_pbase + (uint)(addr - (u8 __force *)dpram_vbase));
}
EXPORT_SYMBOL(cpm_dpram_addr);
+
+struct cpm_ioport16 {
+ __be16 dir, par, sor, dat, intr;
+ __be16 res[3];
+};
+
+struct cpm_ioport32 {
+ __be32 dir, par, sor;
+};
+
+static void cpm1_set_pin32(int port, int pin, int flags)
+{
+ struct cpm_ioport32 __iomem *iop;
+ pin = 1 (31 - pin);
+
+ if (port == 1)
+ iop = (struct cpm_ioport32 __iomem *)
+ mpc8xx_immr-im_cpm.cp_pbdir;
+ else
+ iop = (struct cpm_ioport32 __iomem *)
+ mpc8xx_immr-im_cpm.cp_pedir;
+
+ if (flags CPM_PIN_OUTPUT)
+ setbits32(iop-dir, pin);
+ else
+ clrbits32(iop-dir, pin);
+
+ if (!(flags CPM_PIN_GPIO))
+ setbits32(iop-par, pin);
+ else
+ clrbits32(iop-par, pin);
+
+ if (port == 4) {
+ if (flags CPM_PIN_SECONDARY)
+ setbits32(iop-sor, pin);
+ else
+ clrbits32(iop-sor, pin);
+
+ if (flags CPM_PIN_OPENDRAIN)
+ setbits32(mpc8xx_immr-im_cpm.cp_peodr, pin);
+ else
+ clrbits32(mpc8xx_immr-im_cpm.cp_peodr, pin);
+ }
+}
+
+static void cpm1_set_pin16(int port, int pin, int flags)
+{
+ struct cpm_ioport16 __iomem *iop =
+ (struct cpm_ioport16 __iomem *)mpc8xx_immr-im_ioport;
+
+ pin = 1 (15 - pin);
+
+ if (port != 0)
+ iop += port - 1;
+
+ if (flags CPM_PIN_OUTPUT)
+ setbits16(iop-dir, pin);
+ else
+ clrbits16(iop-dir, pin);
+
+ if (!(flags CPM_PIN_GPIO))
+ setbits16(iop-par, pin);
+ else
+ clrbits16(iop-par, pin);
+
+ if (port == 2) {
+ if (flags CPM_PIN_SECONDARY)
+ setbits16(iop-sor, pin);
+ else
+ clrbits16(iop-sor, pin);
+ }
+}
+
+void cpm1_set_pin(int port, int pin, int flags)
+{
+ if (port == 1 || port == 4)
+ cpm1_set_pin32(port, pin, flags);
+ else
+ cpm1_set_pin16(port, pin, flags);
+}
+
+int cpm1_clk_setup(enum cpm_clk_target target, int clock, int mode)
+{
+ int shift;
+ int i, bits = 0;
+ u32 __iomem *reg;
+ u32 mask = 7;
+
+ u8 clk_map[][3] = {
+ {CPM_CLK_SCC1, CPM_BRG1, 0},
+ {CPM_CLK_SCC1, CPM_BRG2, 1},
+ {CPM_CLK_SCC1, CPM_BRG3, 2},
+ {CPM_CLK_SCC1, CPM_BRG4, 3},
+ {CPM_CLK_SCC1, CPM_CLK1, 4},
+ {CPM_CLK_SCC1, CPM_CLK2, 5},
+ {CPM_CLK_SCC1, CPM_CLK3, 6},
+ {CPM_CLK_SCC1, CPM_CLK4, 7},
+
+ {CPM_CLK_SCC2, CPM_BRG1, 0},
+ {CPM_CLK_SCC2, CPM_BRG2, 1},
+ {CPM_CLK_SCC2, CPM_BRG3, 2},
+ {CPM_CLK_SCC2, CPM_BRG4, 3},
+ {CPM_CLK_SCC2, CPM_CLK1, 4},
+ {CPM_CLK_SCC2, CPM_CLK2, 5},
+ {CPM_CLK_SCC2, CPM_CLK3, 6},
+ {CPM_CLK_SCC2, CPM_CLK4, 7},
+
+ {CPM_CLK_SCC3, CPM_BRG1, 0},
+ {CPM_CLK_SCC3, CPM_BRG2, 1},
+ {CPM_CLK_SCC3, CPM_BRG3, 2},
+ {CPM_CLK_SCC3, CPM_BRG4, 3},
+ {CPM_CLK_SCC3, CPM_CLK5, 4},
+ {CPM_CLK_SCC3, CPM_CLK6, 5},
+ {CPM_CLK_SCC3, CPM_CLK7, 6},
+ {CPM_CLK_SCC3, CPM_CLK8, 7},
+
+ {CPM_CLK_SCC4, CPM_BRG1, 0},
+ {CPM_CLK_SCC4, CPM_BRG2, 1},
+ {CPM_CLK_SCC4, CPM_BRG3, 2},
+ {CPM_CLK_SCC4, CPM_BRG4, 3},
+ {CPM_CLK_SCC4, CPM_CLK5, 4},
+ {CPM_CLK_SCC4, CPM_CLK6, 5},
+ {CPM_CLK_SCC4, CPM_CLK7, 6},
+ {CPM_CLK_SCC4, CPM_CLK8, 7},
+
+ {CPM_CLK_SMC1, CPM_BRG1, 0},
+ {CPM_CLK_SMC1, CPM_BRG2, 1},
+ {CPM_CLK_SMC1, CPM_BRG3, 2},
+ {CPM_CLK_SMC1, CPM_BRG4, 3},
+ {CPM_CLK_SMC1, CPM_CLK1, 4},
+ {CPM_CLK_SMC1, CPM_CLK2, 5},
+ {CPM_CLK_SMC1, CPM_CLK3, 6},
+ {CPM_CLK_SMC1, CPM_CLK4, 7},
+
+ {CPM_CLK_SMC2, CPM_BRG1, 0},
+ {CPM_CLK_SMC2, CPM_BRG2, 1},
+ {CPM_CLK_SMC2, CPM_BRG3, 2},
+
