From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from moutng.kundenserver.de (moutng.kundenserver.de [212.227.126.188]) by ozlabs.org (Postfix) with ESMTP id 285F9DE13D for ; Thu, 23 Apr 2009 23:54:20 +1000 (EST) From: Arnd Bergmann To: linuxppc-dev@ozlabs.org Subject: Re: removing get_immrbase()?? Date: Thu, 23 Apr 2009 15:53:55 +0200 References: In-Reply-To: MIME-Version: 1.0 Message-Id: <200904231553.56464.arnd@arndb.de> Content-Type: multipart/alternative; boundary="Boundary-00=_0LH8JFzC5XDbHj5" Cc: Scott Wood List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , --Boundary-00=_0LH8JFzC5XDbHj5 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable On Wednesday 22 April 2009, Kumar Gala wrote: =46irst of all, thanks for bringing this up, I'd love to see get_immrbase()= gone. > arch/powerpc/sysdev/cpm1.c: mpc8xx_immr =3D ioremap(get_immrbase(), = =20 > 0x4000); > not sure? ideas? Nobody has commented on this, so I've taken a brief look at it. I'd suggest moving the logic up one step at a time. im_cpm, im_siu_conf and im_ioport could be defined locally in sysdev/cpm1.c rather than through mpc8xx_immr, all you need for this is to export accessor functions from cpm= 1 for iop_pcso and cp_cptr: void cpm1_set_iop_pcso(u16 pcso) { setbits16(cpm1_ioport.iop_pcso, pcso); } void cpm1_clear_iop_pcso(u16 pcso) { clearbits16(cpm1_ioport.iop_pcso, pcso); } =2E.. im_sit, im_sitk, im_clkrst and im_clkrstk should be defined locally in m8xx= _setup.c, which is the only place that they are used in. Fortunately, the are all con= tiguous in the address sapce, so they can be moved into one new data structure with= =20 a single static pointer to it in m8xx_setup.c: static struct { struct=AD sys_int_timers sit; struct clk_and_reset clkrst; struct sitk sitk; struct cark clkrstk; } *m8xx_setup_regs; When this is done, 8xx_immap.h along with all the unused stuff therein can = be removed. In the last step, the device trees can be cleaned up so that you can of_iom= ap the regions in those two files directly. Arnd <>< --Boundary-00=_0LH8JFzC5XDbHj5 Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable

On Wednesda= y 22 April 2009, Kumar Gala wrote:

First of al= l, thanks for bringing this up, I'd love to see get_immrbase() gone.

> arch/p= owerpc/sysdev/cpm1.c:=A0=A0=A0=A0=A0mpc8xx_immr =3D ioremap(get_immrbase(),= =A0

> 0x4000= );

> =A0=A0= =A0=A0=A0=A0=A0=A0not sure? ideas?

Nobody has = commented on this, so I've taken a brief look at it.

I'd suggest= moving the logic up one step at a time. im_cpm, im_siu_conf and

im_ioport c= ould be defined locally in sysdev/cpm1.c rather than through

mpc8xx_immr= , all you need for this is to export accessor functions from cpm1 for

iop_pcso an= d cp_cptr:

void cpm1_s= et_iop_pcso(u16 pcso)

{

setbits16(= cpm1_ioport.iop_pcso, pcso);

}

void cpm1_c= lear_iop_pcso(u16 pcso)

{

clearbits1= 6(cpm1_ioport.iop_pcso, pcso);

}

...

im_sit, im_= sitk, im_clkrst and im_clkrstk should be defined locally in m8xx_setup.c,

which is th= e only place that they are used in. Fortunately, the are all contiguous

in the addr= ess sapce, so they can be moved into one new data structure with

a single st= atic pointer to it in m8xx_setup.c:

static stru= ct {

struct=AD = sys_int_timers sit;

struct clk= _and_reset clkrst;

struct sit= k sitk;

struct car= k clkrstk;

} *m8xx_set= up_regs;

When this i= s done, 8xx_immap.h along with all the unused stuff therein can be removed.=

In the last= step, the device trees can be cleaned up so that you can of_iomap

the regions= in those two files directly.

Arnd <&= gt;<

--Boundary-00=_0LH8JFzC5XDbHj5--