diff for duplicates of <20100430121947.1d265ca6@wker> diff --git a/a/1.txt b/N1/1.txt index bf28c25..15edeb1 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -2,23 +2,28 @@ On Thu, 29 Apr 2010 21:05:26 -0500 Timur Tabi <timur.tabi@gmail.com> wrote: > On Thu, Apr 29, 2010 at 6:49 PM, Anatolij Gustschin <agust@denx.de> wrote: -> -> +>=20 +>=20 > > +void __init mpc5121_ads_init_early(void) > > +{ -> > + mpc512x_init_diu(); +> > + =C2=A0 =C2=A0 =C2=A0 mpc512x_init_diu(); > > +} > > + -> > define_machine(mpc5121_ads) { -> > .name = "MPC5121 ADS", -> > .probe = mpc5121_ads_probe, -> > .setup_arch = mpc5121_ads_setup_arch, -> > .init = mpc512x_init, -> > + .init_early = mpc5121_ads_init_early, -> +> > =C2=A0define_machine(mpc5121_ads) { +> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.name =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= +=A0 =C2=A0 =C2=A0 =C2=A0 =3D "MPC5121 ADS", +> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.probe =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = +=C2=A0 =C2=A0 =C2=A0 =C2=A0=3D mpc5121_ads_probe, +> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.setup_arch =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= +=A0 =C2=A0 =3D mpc5121_ads_setup_arch, +> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.init =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= +=A0 =C2=A0 =C2=A0 =C2=A0 =3D mpc512x_init, +> > + =C2=A0 =C2=A0 =C2=A0 .init_early =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = +=C2=A0 =3D mpc5121_ads_init_early, +>=20 > How about just doing this? -> -> .init_early = mpc512x_init_diu, +>=20 +> .init_early =3D mpc512x_init_diu, I thought it should be prepared for adding other code here. mpc5121_ads_init_early() is generic and could contain other @@ -26,111 +31,126 @@ things as well. I would vote for current version. > > +void __init mpc512x_generic_init_early(void) > > +{ -> > + mpc512x_init_diu(); +> > + =C2=A0 =C2=A0 =C2=A0 mpc512x_init_diu(); > > +} > > + > > +void __init mpc512x_generic_setup_arch(void) > > +{ -> > + mpc512x_setup_diu(); +> > + =C2=A0 =C2=A0 =C2=A0 mpc512x_setup_diu(); > > +} > > + -> > define_machine(mpc5121_generic) { -> > .name = "MPC5121 generic", -> > .probe = mpc5121_generic_probe, -> > .init = mpc512x_init, -> > + .init_early = mpc512x_generic_init_early, -> > + .setup_arch = mpc512x_generic_setup_arch, -> +> > =C2=A0define_machine(mpc5121_generic) { +> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.name =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= +=A0 =C2=A0 =C2=A0 =C2=A0 =3D "MPC5121 generic", +> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.probe =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = +=C2=A0 =C2=A0 =C2=A0 =C2=A0=3D mpc5121_generic_probe, +> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.init =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= +=A0 =C2=A0 =C2=A0 =C2=A0 =3D mpc512x_init, +> > + =C2=A0 =C2=A0 =C2=A0 .init_early =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = +=C2=A0 =3D mpc512x_generic_init_early, +> > + =C2=A0 =C2=A0 =C2=A0 .setup_arch =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = +=C2=A0 =3D mpc512x_generic_setup_arch, +>=20 > And a similar change here. Same here. ... -> > diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c +> > diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerp= +c/platforms/512x/mpc512x_shared.c > > index b7f518a..8e297fa 100644 > > --- a/arch/powerpc/platforms/512x/mpc512x_shared.c > > +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c > > @@ -16,7 +16,11 @@ -> > #include <linux/io.h> -> > #include <linux/irq.h> -> > #include <linux/of_platform.h> +> > =C2=A0#include <linux/io.h> +> > =C2=A0#include <linux/irq.h> +> > =C2=A0#include <linux/of_platform.h> > > +#include <linux/fsl-diu-fb.h> > > +#include <linux/bootmem.h> > > +#include <sysdev/fsl_soc.h> > > > > +#include <asm/cacheflush.h> -> > #include <asm/machdep.h> -> > #include <asm/ipic.h> -> > #include <asm/prom.h> +> > =C2=A0#include <asm/machdep.h> +> > =C2=A0#include <asm/ipic.h> +> > =C2=A0#include <asm/prom.h> > > @@ -53,6 +57,286 @@ void mpc512x_restart(char *cmd) -> > ; -> > } +> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0; +> > =C2=A0} > > > > +struct fsl_diu_shared_fb { -> > + char gamma[0x300]; /* 32-bit aligned! */ -> +> > + =C2=A0 =C2=A0 =C2=A0 char =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ga= +mma[0x300]; =C2=A0 /* 32-bit aligned! */ +>=20 > char or u8? Will use u8. -> > + struct diu_ad ad0; /* 32-bit aligned! */ -> > + phys_addr_t fb_phys; -> > + size_t fb_len; -> > + bool in_use; +> > + =C2=A0 =C2=A0 =C2=A0 struct diu_ad =C2=A0 ad0; =C2=A0 =C2=A0 =C2=A0 = +=C2=A0 =C2=A0 =C2=A0/* 32-bit aligned! */ +> > + =C2=A0 =C2=A0 =C2=A0 phys_addr_t =C2=A0 =C2=A0 fb_phys; +> > + =C2=A0 =C2=A0 =C2=A0 size_t =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0fb_len; +> > + =C2=A0 =C2=A0 =C2=A0 bool =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0in= +_use; > > +}; -> +>=20 > Where did "bool" come from? Use "int" instead. It is common practise to use "bool" type, grep in drivers dir. > > +unsigned int mpc512x_get_pixel_format(unsigned int bits_per_pixel, -> > + int monitor_port) +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 int monitor_port) > > +{ -> > + unsigned int pix_fmt; +> > + =C2=A0 =C2=A0 =C2=A0 unsigned int pix_fmt; > > + -> > + switch (bits_per_pixel) { -> > + case 32: -> > + pix_fmt = 0x88883316; -> > + break; -> > + case 24: -> > + pix_fmt = 0x88082219; -> > + break; -> > + case 16: -> > + pix_fmt = 0x65053118; -> > + break; -> > + default: -> > + pix_fmt = 0x00000400; -> > + } -> > + return pix_fmt; +> > + =C2=A0 =C2=A0 =C2=A0 switch (bits_per_pixel) { +> > + =C2=A0 =C2=A0 =C2=A0 case 32: +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pix_fmt =3D 0x888833= +16; +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break; +> > + =C2=A0 =C2=A0 =C2=A0 case 24: +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pix_fmt =3D 0x880822= +19; +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break; +> > + =C2=A0 =C2=A0 =C2=A0 case 16: +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pix_fmt =3D 0x650531= +18; +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break; +> > + =C2=A0 =C2=A0 =C2=A0 default: +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pix_fmt =3D 0x000004= +00; +> > + =C2=A0 =C2=A0 =C2=A0 } +> > + =C2=A0 =C2=A0 =C2=A0 return pix_fmt; > > +} -> +>=20 > This is simpler: -> +>=20 > switch (bits_per_pixel) { > case 32: > return 0x88883316; > case 24: > return 0x88082219; > case 16: -> return = 0x65053118; +> return =3D 0x65053118; > } -> +>=20 > return 0x00000400; > } Will simplify as suggested, thanks! -> > + ccm = of_iomap(np, 0); -> > + if (!ccm) { -> > + pr_err("Can't map clock control module reg.\n"); -> > + of_node_put(np); -> > + return; -> > + } -> > + of_node_put(np); -> +> > + =C2=A0 =C2=A0 =C2=A0 ccm =3D of_iomap(np, 0); +> > + =C2=A0 =C2=A0 =C2=A0 if (!ccm) { +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_err("Can't map cl= +ock control module reg.\n"); +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 of_node_put(np); +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return; +> > + =C2=A0 =C2=A0 =C2=A0 } +> > + =C2=A0 =C2=A0 =C2=A0 of_node_put(np); +>=20 > This is simpler: -> -> ccm = of_iomap(np, 0); +>=20 +> ccm =3D of_iomap(np, 0); > of_node_put(np); > if (!ccm) { > pr_err("Can't map clock control module reg.\n"); @@ -139,105 +159,127 @@ Will simplify as suggested, thanks! OK, will fix, thanks. -> -> > + np = of_find_node_by_type(NULL, "cpu"); -> > + if (np) { -> > + unsigned int size; -> > + const unsigned int *prop > > + of_get_property(np, "bus-frequency", &size); -> +>=20 +> > + =C2=A0 =C2=A0 =C2=A0 np =3D of_find_node_by_type(NULL, "cpu"); +> > + =C2=A0 =C2=A0 =C2=A0 if (np) { +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 unsigned int size; +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 const unsigned int *= +prop =3D +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= + =C2=A0 of_get_property(np, "bus-frequency", &size); +>=20 > Since you don't use 'size', you can skip it: -> -> const unsigned int *prop > of_get_property(np, "bus-frequency", NULL); -> -> > + } else { -> > + pr_err("Can't find \"cpu\" node.\n"); -> +>=20 +> const unsigned int *prop =3D +> of_get_property(np, "bus-frequency", NULL); +>=20 +> > + =C2=A0 =C2=A0 =C2=A0 } else { +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_err("Can't find \= +"cpu\" node.\n"); +>=20 > 'cpu' is simpler than \"cpu\" Will simplify, too. -> > + /* Calculate the pixel clock with the smallest error */ -> > + /* calculate the following in steps to avoid overflow */ -> > + pr_debug("DIU pixclock in ps - %d\n", pixclock); -> > + temp = (1000000000 / pixclock) * 1000; -> +> > + =C2=A0 =C2=A0 =C2=A0 /* Calculate the pixel clock with the smallest e= +rror */ +> > + =C2=A0 =C2=A0 =C2=A0 /* calculate the following in steps to avoid ove= +rflow */ +> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU pixclock in ps - %d\n", pixclock); +> > + =C2=A0 =C2=A0 =C2=A0 temp =3D (1000000000 / pixclock) * 1000; +>=20 > I'm pretty sure the compiler will optimize this to: -> -> temp = (1000000000000UL / pixclock); -> +>=20 +> temp =3D (1000000000000UL / pixclock); +>=20 > so you may as well do it that way. ?? 1000000000000 is _not_ UL, but UUL. -> -> > + pixclock = temp; -> > + pr_debug("DIU pixclock freq - %u\n", pixclock); +>=20 +> > + =C2=A0 =C2=A0 =C2=A0 pixclock =3D temp; +> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU pixclock freq - %u\n", pixclock); > > + -> > + temp = (temp * 5) / 100; /* pixclock * 0.05 */ -> +> > + =C2=A0 =C2=A0 =C2=A0 temp =3D (temp * 5) / 100; /* pixclock * 0.05 */ +>=20 > The compiler will optimize this to: -> -> temp /= 20; +>=20 +> temp /=3D 20; Can do it, too. Thanks. -> > + pr_debug("deviation = %d\n", temp); -> > + minpixclock = pixclock - temp; -> > + maxpixclock = pixclock + temp; -> > + pr_debug("DIU minpixclock - %lu\n", minpixclock); -> > + pr_debug("DIU maxpixclock - %lu\n", maxpixclock); -> > + pixval = speed_ccb/pixclock; -> > + pr_debug("DIU pixval = %lu\n", pixval); +> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("deviation =3D %d\n", temp); +> > + =C2=A0 =C2=A0 =C2=A0 minpixclock =3D pixclock - temp; +> > + =C2=A0 =C2=A0 =C2=A0 maxpixclock =3D pixclock + temp; +> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU minpixclock - %lu\n", minpixclock); +> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU maxpixclock - %lu\n", maxpixclock); +> > + =C2=A0 =C2=A0 =C2=A0 pixval =3D speed_ccb/pixclock; +> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU pixval =3D %lu\n", pixval); > > + -> > + err = 100000000; -> +> > + =C2=A0 =C2=A0 =C2=A0 err =3D 100000000; +>=20 > Why do you assign err to this arbitrary value? Dunno. It is Freescale's code and I do not have time to check and understand each bit of it and to explain it. -> > + bestval = pixval; -> > + pr_debug("DIU bestval = %lu\n", bestval); +> > + =C2=A0 =C2=A0 =C2=A0 bestval =3D pixval; +> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU bestval =3D %lu\n", bestval); > > + -> > + bestfreq = 0; -> > + for (i = -1; i <= 1; i++) { -> > + temp = speed_ccb / (pixval+i); -> > + pr_debug("DIU test pixval i=%d, pixval=%lu, temp freq. = %u\n", -> > + i, pixval, temp); -> > + if ((temp < minpixclock) || (temp > maxpixclock)) -> > + pr_debug("DIU exceeds monitor range (%lu to %lu)\n", -> > + minpixclock, maxpixclock); -> > + else if (abs(temp - pixclock) < err) { -> > + pr_debug("Entered the else if block %d\n", i); -> > + err = abs(temp - pixclock); -> > + bestval = pixval + i; -> > + bestfreq = temp; -> > + } -> > + } +> > + =C2=A0 =C2=A0 =C2=A0 bestfreq =3D 0; +> > + =C2=A0 =C2=A0 =C2=A0 for (i =3D -1; i <=3D 1; i++) { +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 temp =3D speed_ccb /= + (pixval+i); +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU test p= +ixval i=3D%d, pixval=3D%lu, temp freq. =3D %u\n", +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= + =C2=A0 i, pixval, temp); +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if ((temp < minpixcl= +ock) || (temp > maxpixclock)) +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= + =C2=A0 pr_debug("DIU exceeds monitor range (%lu to %lu)\n", +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 minpixclock, maxpixclock); +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 else if (abs(temp - = +pixclock) < err) { +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= + =C2=A0 pr_debug("Entered the else if block %d\n", i); +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= + =C2=A0 err =3D abs(temp - pixclock); +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= + =C2=A0 bestval =3D pixval + i; +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= + =C2=A0 bestfreq =3D temp; +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 } +> > + =C2=A0 =C2=A0 =C2=A0 } > > + -> > + pr_debug("DIU chose = %lx\n", bestval); -> > + pr_debug("DIU error = %ld\n NomPixClk ", err); -> > + pr_debug("DIU: Best Freq = %lx\n", bestfreq); -> > + /* Modify DIU_DIV in CCM SCFR1 */ -> > + temp = in_be32(ccm + CCM_SCFR1); -> -> Don't use offsets like + CCM_SCFR1. Create a structure and use that instead. +> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU chose =3D %lx\n", bestval); +> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU error =3D %ld\n NomPixClk ", err); +> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU: Best Freq =3D %lx\n", bestfreq); +> > + =C2=A0 =C2=A0 =C2=A0 /* Modify DIU_DIV in CCM SCFR1 */ +> > + =C2=A0 =C2=A0 =C2=A0 temp =3D in_be32(ccm + CCM_SCFR1); +>=20 +> Don't use offsets like + CCM_SCFR1. Create a structure and use that inst= +ead. Done in next patch version. -> > + pr_debug("DIU: Current value of SCFR1: 0x%08x\n", temp); -> > + temp &= ~DIU_DIV_MASK; -> > + temp |= (bestval & DIU_DIV_MASK); -> > + out_be32(ccm + CCM_SCFR1, temp); -> > + pr_debug("DIU: Modified value of SCFR1: 0x%08x\n", temp); -> > + iounmap(ccm); +> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU: Current value of SCFR1: 0x%08x\n"= +, temp); +> > + =C2=A0 =C2=A0 =C2=A0 temp &=3D ~DIU_DIV_MASK; +> > + =C2=A0 =C2=A0 =C2=A0 temp |=3D (bestval & DIU_DIV_MASK); +> > + =C2=A0 =C2=A0 =C2=A0 out_be32(ccm + CCM_SCFR1, temp); +> > + =C2=A0 =C2=A0 =C2=A0 pr_debug("DIU: Modified value of SCFR1: 0x%08x\n= +", temp); +> > + =C2=A0 =C2=A0 =C2=A0 iounmap(ccm); > > +} > > + > > +ssize_t mpc512x_show_monitor_port(int monitor_port, char *buf) > > +{ -> > + return snprintf(buf, PAGE_SIZE, "0 - 5121 LCD\n"); -> +> > + =C2=A0 =C2=A0 =C2=A0 return snprintf(buf, PAGE_SIZE, "0 - 5121 LCD\n"= +); +>=20 > There's no point in using snprintf since you're printing a string > literal. You can use sprintf. @@ -247,42 +289,49 @@ Will do, thanks. > > + > > +int mpc512x_set_sysfs_monitor_port(int val) > > +{ -> > + return 0; +> > + =C2=A0 =C2=A0 =C2=A0 return 0; > > +} > > + -> > +static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_shared_fb; +> > +static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_s= +hared_fb; > > + > > +#if defined(CONFIG_FB_FSL_DIU) || \ -> > + defined(CONFIG_FB_FSL_DIU_MODULE) +> > + =C2=A0 =C2=A0defined(CONFIG_FB_FSL_DIU_MODULE) > > +static inline void mpc512x_free_bootmem(struct page *page) > > +{ -> > + __ClearPageReserved(page); -> > + BUG_ON(PageTail(page)); -> > + BUG_ON(atomic_read(&page->_count) > 1); -> > + atomic_set(&page->_count, 1); -> > + __free_page(page); -> > + totalram_pages++; +> > + =C2=A0 =C2=A0 =C2=A0 __ClearPageReserved(page); +> > + =C2=A0 =C2=A0 =C2=A0 BUG_ON(PageTail(page)); +> > + =C2=A0 =C2=A0 =C2=A0 BUG_ON(atomic_read(&page->_count) > 1); +> > + =C2=A0 =C2=A0 =C2=A0 atomic_set(&page->_count, 1); +> > + =C2=A0 =C2=A0 =C2=A0 __free_page(page); +> > + =C2=A0 =C2=A0 =C2=A0 totalram_pages++; > > +} > > + > > +void mpc512x_release_bootmem(void) > > +{ -> > + unsigned long addr = diu_shared_fb.fb_phys & PAGE_MASK; -> > + unsigned long size = diu_shared_fb.fb_len; -> > + unsigned long start, end; +> > + =C2=A0 =C2=A0 =C2=A0 unsigned long addr =3D diu_shared_fb.fb_phys & P= +AGE_MASK; +> > + =C2=A0 =C2=A0 =C2=A0 unsigned long size =3D diu_shared_fb.fb_len; +> > + =C2=A0 =C2=A0 =C2=A0 unsigned long start, end; > > + -> > + if (diu_shared_fb.in_use) { -> > + start = PFN_UP(addr); -> > + end = PFN_DOWN(addr + size); +> > + =C2=A0 =C2=A0 =C2=A0 if (diu_shared_fb.in_use) { +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 start =3D PFN_UP(add= +r); +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 end =3D PFN_DOWN(add= +r + size); > > + -> > + for (; start < end; start++) -> > + mpc512x_free_bootmem(pfn_to_page(start)); +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for (; start < end; = +start++) +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= + =C2=A0 mpc512x_free_bootmem(pfn_to_page(start)); > > + -> > + diu_shared_fb.in_use = false; -> > + } -> > + diu_ops.release_bootmem = NULL; +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 diu_shared_fb.in_use= + =3D false; +> > + =C2=A0 =C2=A0 =C2=A0 } +> > + =C2=A0 =C2=A0 =C2=A0 diu_ops.release_bootmem =3D NULL; > > +} > > +#endif -> +>=20 > Do you really need to use reserve_bootmem? Have you tried kmalloc or > alloc_pages_exact()? @@ -299,35 +348,38 @@ Yes. No, it is too early to use them here. > > + */ > > +void __init mpc512x_init_diu(void) > > +{ -> > + struct device_node *np; -> > + void __iomem *diu_reg; -> > + phys_addr_t desc; -> > + void __iomem *vaddr; -> > + unsigned long mode, pix_fmt, res, bpp; -> > + unsigned long dst; +> > + =C2=A0 =C2=A0 =C2=A0 struct device_node *np; +> > + =C2=A0 =C2=A0 =C2=A0 void __iomem *diu_reg; +> > + =C2=A0 =C2=A0 =C2=A0 phys_addr_t desc; +> > + =C2=A0 =C2=A0 =C2=A0 void __iomem *vaddr; +> > + =C2=A0 =C2=A0 =C2=A0 unsigned long mode, pix_fmt, res, bpp; +> > + =C2=A0 =C2=A0 =C2=A0 unsigned long dst; > > + -> > + np = of_find_compatible_node(NULL, NULL, "fsl,mpc5121-diu"); -> > + if (!np) { -> > + pr_err("No DIU node\n"); -> > + return; -> > + } -> +> > + =C2=A0 =C2=A0 =C2=A0 np =3D of_find_compatible_node(NULL, NULL, "fsl,= +mpc5121-diu"); +> > + =C2=A0 =C2=A0 =C2=A0 if (!np) { +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_err("No DIU node\= +n"); +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return; +> > + =C2=A0 =C2=A0 =C2=A0 } +>=20 > Shouldn't you be probing as an OF driver instead of manually searching > for the DIU node? No, not here. > > + -> > + diu_reg = of_iomap(np, 0); -> > + of_node_put(np); -> > + if (!diu_reg) { -> > + pr_err("Can't map DIU\n"); -> > + return; -> > + } +> > + =C2=A0 =C2=A0 =C2=A0 diu_reg =3D of_iomap(np, 0); +> > + =C2=A0 =C2=A0 =C2=A0 of_node_put(np); +> > + =C2=A0 =C2=A0 =C2=A0 if (!diu_reg) { +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_err("Can't map DI= +U\n"); +> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return; +> > + =C2=A0 =C2=A0 =C2=A0 } > > + -> > + mode = in_be32(diu_reg + 0x1c); -> > + if (mode != 1) { -> +> > + =C2=A0 =C2=A0 =C2=A0 mode =3D in_be32(diu_reg + 0x1c); +> > + =C2=A0 =C2=A0 =C2=A0 if (mode !=3D 1) { +>=20 > How can in_be32() return a -1? It is a 1, not -1. I will use appropriate macro here and also diff --git a/a/content_digest b/N1/content_digest index 1eb84f7..5b33e3e 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -3,7 +3,7 @@ "ref\0s2ted82fe3e1004291905mf562f0cbi24054bb973aa2dbb@mail.gmail.com\0" "From\0Anatolij Gustschin <agust@denx.de>\0" "Subject\0Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support\0" - "Date\0Fri, 30 Apr 2010 10:19:47 +0000\0" + "Date\0Fri, 30 Apr 2010 12:19:47 +0200\0" "To\0Timur Tabi <timur.tabi@gmail.com>\0" "Cc\0linux-fbdev@vger.kernel.org" wd@denx.de @@ -18,23 +18,28 @@ "Timur Tabi <timur.tabi@gmail.com> wrote:\n" "\n" "> On Thu, Apr 29, 2010 at 6:49 PM, Anatolij Gustschin <agust@denx.de> wrote:\n" - "> \n" - "> \n" + ">=20\n" + ">=20\n" "> > +void __init mpc5121_ads_init_early(void)\n" "> > +{\n" - "> > + \302\240 \302\240 \302\240 mpc512x_init_diu();\n" + "> > + =C2=A0 =C2=A0 =C2=A0 mpc512x_init_diu();\n" "> > +}\n" "> > +\n" - "> > \302\240define_machine(mpc5121_ads) {\n" - "> > \302\240 \302\240 \302\240 \302\240.name \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 = \"MPC5121 ADS\",\n" - "> > \302\240 \302\240 \302\240 \302\240.probe \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240= mpc5121_ads_probe,\n" - "> > \302\240 \302\240 \302\240 \302\240.setup_arch \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 = mpc5121_ads_setup_arch,\n" - "> > \302\240 \302\240 \302\240 \302\240.init \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 = mpc512x_init,\n" - "> > + \302\240 \302\240 \302\240 .init_early \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 = mpc5121_ads_init_early,\n" - "> \n" + "> > =C2=A0define_machine(mpc5121_ads) {\n" + "> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.name =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=\n" + "=A0 =C2=A0 =C2=A0 =C2=A0 =3D \"MPC5121 ADS\",\n" + "> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.probe =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =\n" + "=C2=A0 =C2=A0 =C2=A0 =C2=A0=3D mpc5121_ads_probe,\n" + "> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.setup_arch =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=\n" + "=A0 =C2=A0 =3D mpc5121_ads_setup_arch,\n" + "> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.init =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=\n" + "=A0 =C2=A0 =C2=A0 =C2=A0 =3D mpc512x_init,\n" + "> > + =C2=A0 =C2=A0 =C2=A0 .init_early =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =\n" + "=C2=A0 =3D mpc5121_ads_init_early,\n" + ">=20\n" "> How about just doing this?\n" - "> \n" - "> .init_early = mpc512x_init_diu,\n" + ">=20\n" + "> .init_early =3D mpc512x_init_diu,\n" "\n" "I thought it should be prepared for adding other code here.\n" "mpc5121_ads_init_early() is generic and could contain other\n" @@ -42,111 +47,126 @@ "\n" "> > +void __init mpc512x_generic_init_early(void)\n" "> > +{\n" - "> > + \302\240 \302\240 \302\240 mpc512x_init_diu();\n" + "> > + =C2=A0 =C2=A0 =C2=A0 mpc512x_init_diu();\n" "> > +}\n" "> > +\n" "> > +void __init mpc512x_generic_setup_arch(void)\n" "> > +{\n" - "> > + \302\240 \302\240 \302\240 mpc512x_setup_diu();\n" + "> > + =C2=A0 =C2=A0 =C2=A0 mpc512x_setup_diu();\n" "> > +}\n" "> > +\n" - "> > \302\240define_machine(mpc5121_generic) {\n" - "> > \302\240 \302\240 \302\240 \302\240.name \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 = \"MPC5121 generic\",\n" - "> > \302\240 \302\240 \302\240 \302\240.probe \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240= mpc5121_generic_probe,\n" - "> > \302\240 \302\240 \302\240 \302\240.init \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 = mpc512x_init,\n" - "> > + \302\240 \302\240 \302\240 .init_early \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 = mpc512x_generic_init_early,\n" - "> > + \302\240 \302\240 \302\240 .setup_arch \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 = mpc512x_generic_setup_arch,\n" - "> \n" + "> > =C2=A0define_machine(mpc5121_generic) {\n" + "> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.name =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=\n" + "=A0 =C2=A0 =C2=A0 =C2=A0 =3D \"MPC5121 generic\",\n" + "> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.probe =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =\n" + "=C2=A0 =C2=A0 =C2=A0 =C2=A0=3D mpc5121_generic_probe,\n" + "> > =C2=A0 =C2=A0 =C2=A0 =C2=A0.init =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=\n" + "=A0 =C2=A0 =C2=A0 =C2=A0 =3D mpc512x_init,\n" + "> > + =C2=A0 =C2=A0 =C2=A0 .init_early =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =\n" + "=C2=A0 =3D mpc512x_generic_init_early,\n" + "> > + =C2=A0 =C2=A0 =C2=A0 .setup_arch =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =\n" + "=C2=A0 =3D mpc512x_generic_setup_arch,\n" + ">=20\n" "> And a similar change here.\n" "\n" "Same here.\n" "\n" "...\n" - "> > diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c\n" + "> > diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerp=\n" + "c/platforms/512x/mpc512x_shared.c\n" "> > index b7f518a..8e297fa 100644\n" "> > --- a/arch/powerpc/platforms/512x/mpc512x_shared.c\n" "> > +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c\n" "> > @@ -16,7 +16,11 @@\n" - "> > \302\240#include <linux/io.h>\n" - "> > \302\240#include <linux/irq.h>\n" - "> > \302\240#include <linux/of_platform.h>\n" + "> > =C2=A0#include <linux/io.h>\n" + "> > =C2=A0#include <linux/irq.h>\n" + "> > =C2=A0#include <linux/of_platform.h>\n" "> > +#include <linux/fsl-diu-fb.h>\n" "> > +#include <linux/bootmem.h>\n" "> > +#include <sysdev/fsl_soc.h>\n" "> >\n" "> > +#include <asm/cacheflush.h>\n" - "> > \302\240#include <asm/machdep.h>\n" - "> > \302\240#include <asm/ipic.h>\n" - "> > \302\240#include <asm/prom.h>\n" + "> > =C2=A0#include <asm/machdep.h>\n" + "> > =C2=A0#include <asm/ipic.h>\n" + "> > =C2=A0#include <asm/prom.h>\n" "> > @@ -53,6 +57,286 @@ void mpc512x_restart(char *cmd)\n" - "> > \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240;\n" - "> > \302\240}\n" + "> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0;\n" + "> > =C2=A0}\n" "> >\n" "> > +struct fsl_diu_shared_fb {\n" - "> > + \302\240 \302\240 \302\240 char \302\240 \302\240 \302\240 \302\240 \302\240 \302\240gamma[0x300]; \302\240 /* 32-bit aligned! */\n" - "> \n" + "> > + =C2=A0 =C2=A0 =C2=A0 char =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ga=\n" + "mma[0x300]; =C2=A0 /* 32-bit aligned! */\n" + ">=20\n" "> char or u8?\n" "\n" "Will use u8.\n" "\n" - "> > + \302\240 \302\240 \302\240 struct diu_ad \302\240 ad0; \302\240 \302\240 \302\240 \302\240 \302\240 \302\240/* 32-bit aligned! */\n" - "> > + \302\240 \302\240 \302\240 phys_addr_t \302\240 \302\240 fb_phys;\n" - "> > + \302\240 \302\240 \302\240 size_t \302\240 \302\240 \302\240 \302\240 \302\240fb_len;\n" - "> > + \302\240 \302\240 \302\240 bool \302\240 \302\240 \302\240 \302\240 \302\240 \302\240in_use;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 struct diu_ad =C2=A0 ad0; =C2=A0 =C2=A0 =C2=A0 =\n" + "=C2=A0 =C2=A0 =C2=A0/* 32-bit aligned! */\n" + "> > + =C2=A0 =C2=A0 =C2=A0 phys_addr_t =C2=A0 =C2=A0 fb_phys;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 size_t =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0fb_len;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 bool =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0in=\n" + "_use;\n" "> > +};\n" - "> \n" + ">=20\n" "> Where did \"bool\" come from? Use \"int\" instead.\n" "\n" "It is common practise to use \"bool\" type, grep in drivers dir.\n" "\n" "> > +unsigned int mpc512x_get_pixel_format(unsigned int bits_per_pixel,\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 int monitor_port)\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=\n" + " =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 int monitor_port)\n" "> > +{\n" - "> > + \302\240 \302\240 \302\240 unsigned int pix_fmt;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 unsigned int pix_fmt;\n" "> > +\n" - "> > + \302\240 \302\240 \302\240 switch (bits_per_pixel) {\n" - "> > + \302\240 \302\240 \302\240 case 32:\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 pix_fmt = 0x88883316;\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 break;\n" - "> > + \302\240 \302\240 \302\240 case 24:\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 pix_fmt = 0x88082219;\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 break;\n" - "> > + \302\240 \302\240 \302\240 case 16:\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 pix_fmt = 0x65053118;\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 break;\n" - "> > + \302\240 \302\240 \302\240 default:\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 pix_fmt = 0x00000400;\n" - "> > + \302\240 \302\240 \302\240 }\n" - "> > + \302\240 \302\240 \302\240 return pix_fmt;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 switch (bits_per_pixel) {\n" + "> > + =C2=A0 =C2=A0 =C2=A0 case 32:\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pix_fmt =3D 0x888833=\n" + "16;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 case 24:\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pix_fmt =3D 0x880822=\n" + "19;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 case 16:\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pix_fmt =3D 0x650531=\n" + "18;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 default:\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pix_fmt =3D 0x000004=\n" + "00;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 }\n" + "> > + =C2=A0 =C2=A0 =C2=A0 return pix_fmt;\n" "> > +}\n" - "> \n" + ">=20\n" "> This is simpler:\n" - "> \n" + ">=20\n" "> switch (bits_per_pixel) {\n" "> case 32:\n" "> return 0x88883316;\n" "> case 24:\n" "> return 0x88082219;\n" "> case 16:\n" - "> return = 0x65053118;\n" + "> return =3D 0x65053118;\n" "> }\n" - "> \n" + ">=20\n" "> return 0x00000400;\n" "> }\n" "\n" "Will simplify as suggested, thanks!\n" "\n" - "> > + \302\240 \302\240 \302\240 ccm = of_iomap(np, 0);\n" - "> > + \302\240 \302\240 \302\240 if (!ccm) {\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 pr_err(\"Can't map clock control module reg.\\n\");\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 of_node_put(np);\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 return;\n" - "> > + \302\240 \302\240 \302\240 }\n" - "> > + \302\240 \302\240 \302\240 of_node_put(np);\n" - "> \n" + "> > + =C2=A0 =C2=A0 =C2=A0 ccm =3D of_iomap(np, 0);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 if (!ccm) {\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_err(\"Can't map cl=\n" + "ock control module reg.\\n\");\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 of_node_put(np);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 }\n" + "> > + =C2=A0 =C2=A0 =C2=A0 of_node_put(np);\n" + ">=20\n" "> This is simpler:\n" - "> \n" - "> ccm = of_iomap(np, 0);\n" + ">=20\n" + "> ccm =3D of_iomap(np, 0);\n" "> of_node_put(np);\n" "> if (!ccm) {\n" "> pr_err(\"Can't map clock control module reg.\\n\");\n" @@ -155,105 +175,127 @@ "\n" "OK, will fix, thanks.\n" "\n" - "> \n" - "> > + \302\240 \302\240 \302\240 np = of_find_node_by_type(NULL, \"cpu\");\n" - "> > + \302\240 \302\240 \302\240 if (np) {\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 unsigned int size;\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 const unsigned int *prop > > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 of_get_property(np, \"bus-frequency\", &size);\n" - "> \n" + ">=20\n" + "> > + =C2=A0 =C2=A0 =C2=A0 np =3D of_find_node_by_type(NULL, \"cpu\");\n" + "> > + =C2=A0 =C2=A0 =C2=A0 if (np) {\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 unsigned int size;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 const unsigned int *=\n" + "prop =3D\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=\n" + " =C2=A0 of_get_property(np, \"bus-frequency\", &size);\n" + ">=20\n" "> Since you don't use 'size', you can skip it:\n" - "> \n" - "> const unsigned int *prop > of_get_property(np, \"bus-frequency\", NULL);\n" - "> \n" - "> > + \302\240 \302\240 \302\240 } else {\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 pr_err(\"Can't find \\\"cpu\\\" node.\\n\");\n" - "> \n" + ">=20\n" + "> const unsigned int *prop =3D\n" + "> of_get_property(np, \"bus-frequency\", NULL);\n" + ">=20\n" + "> > + =C2=A0 =C2=A0 =C2=A0 } else {\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_err(\"Can't find \\=\n" + "\"cpu\\\" node.\\n\");\n" + ">=20\n" "> 'cpu' is simpler than \\\"cpu\\\"\n" "\n" "Will simplify, too.\n" "\n" - "> > + \302\240 \302\240 \302\240 /* Calculate the pixel clock with the smallest error */\n" - "> > + \302\240 \302\240 \302\240 /* calculate the following in steps to avoid overflow */\n" - "> > + \302\240 \302\240 \302\240 pr_debug(\"DIU pixclock in ps - %d\\n\", pixclock);\n" - "> > + \302\240 \302\240 \302\240 temp = (1000000000 / pixclock) * 1000;\n" - "> \n" + "> > + =C2=A0 =C2=A0 =C2=A0 /* Calculate the pixel clock with the smallest e=\n" + "rror */\n" + "> > + =C2=A0 =C2=A0 =C2=A0 /* calculate the following in steps to avoid ove=\n" + "rflow */\n" + "> > + =C2=A0 =C2=A0 =C2=A0 pr_debug(\"DIU pixclock in ps - %d\\n\", pixclock);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 temp =3D (1000000000 / pixclock) * 1000;\n" + ">=20\n" "> I'm pretty sure the compiler will optimize this to:\n" - "> \n" - "> temp = (1000000000000UL / pixclock);\n" - "> \n" + ">=20\n" + "> temp =3D (1000000000000UL / pixclock);\n" + ">=20\n" "> so you may as well do it that way.\n" "\n" "??\n" "1000000000000 is _not_ UL, but UUL.\n" "\n" - "> \n" - "> > + \302\240 \302\240 \302\240 pixclock = temp;\n" - "> > + \302\240 \302\240 \302\240 pr_debug(\"DIU pixclock freq - %u\\n\", pixclock);\n" + ">=20\n" + "> > + =C2=A0 =C2=A0 =C2=A0 pixclock =3D temp;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 pr_debug(\"DIU pixclock freq - %u\\n\", pixclock);\n" "> > +\n" - "> > + \302\240 \302\240 \302\240 temp = (temp * 5) / 100; /* pixclock * 0.05 */\n" - "> \n" + "> > + =C2=A0 =C2=A0 =C2=A0 temp =3D (temp * 5) / 100; /* pixclock * 0.05 */\n" + ">=20\n" "> The compiler will optimize this to:\n" - "> \n" - "> temp /= 20;\n" + ">=20\n" + "> temp /=3D 20;\n" "\n" "Can do it, too. Thanks.\n" "\n" - "> > + \302\240 \302\240 \302\240 pr_debug(\"deviation = %d\\n\", temp);\n" - "> > + \302\240 \302\240 \302\240 minpixclock = pixclock - temp;\n" - "> > + \302\240 \302\240 \302\240 maxpixclock = pixclock + temp;\n" - "> > + \302\240 \302\240 \302\240 pr_debug(\"DIU minpixclock - %lu\\n\", minpixclock);\n" - "> > + \302\240 \302\240 \302\240 pr_debug(\"DIU maxpixclock - %lu\\n\", maxpixclock);\n" - "> > + \302\240 \302\240 \302\240 pixval = speed_ccb/pixclock;\n" - "> > + \302\240 \302\240 \302\240 pr_debug(\"DIU pixval = %lu\\n\", pixval);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 pr_debug(\"deviation =3D %d\\n\", temp);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 minpixclock =3D pixclock - temp;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 maxpixclock =3D pixclock + temp;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 pr_debug(\"DIU minpixclock - %lu\\n\", minpixclock);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 pr_debug(\"DIU maxpixclock - %lu\\n\", maxpixclock);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 pixval =3D speed_ccb/pixclock;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 pr_debug(\"DIU pixval =3D %lu\\n\", pixval);\n" "> > +\n" - "> > + \302\240 \302\240 \302\240 err = 100000000;\n" - "> \n" + "> > + =C2=A0 =C2=A0 =C2=A0 err =3D 100000000;\n" + ">=20\n" "> Why do you assign err to this arbitrary value?\n" "\n" "Dunno. It is Freescale's code and I do not have time to check\n" "and understand each bit of it and to explain it.\n" "\n" - "> > + \302\240 \302\240 \302\240 bestval = pixval;\n" - "> > + \302\240 \302\240 \302\240 pr_debug(\"DIU bestval = %lu\\n\", bestval);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 bestval =3D pixval;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 pr_debug(\"DIU bestval =3D %lu\\n\", bestval);\n" "> > +\n" - "> > + \302\240 \302\240 \302\240 bestfreq = 0;\n" - "> > + \302\240 \302\240 \302\240 for (i = -1; i <= 1; i++) {\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 temp = speed_ccb / (pixval+i);\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 pr_debug(\"DIU test pixval i=%d, pixval=%lu, temp freq. = %u\\n\",\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 i, pixval, temp);\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 if ((temp < minpixclock) || (temp > maxpixclock))\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 pr_debug(\"DIU exceeds monitor range (%lu to %lu)\\n\",\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 minpixclock, maxpixclock);\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 else if (abs(temp - pixclock) < err) {\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 pr_debug(\"Entered the else if block %d\\n\", i);\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 err = abs(temp - pixclock);\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 bestval = pixval + i;\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 bestfreq = temp;\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 }\n" - "> > + \302\240 \302\240 \302\240 }\n" + "> > + =C2=A0 =C2=A0 =C2=A0 bestfreq =3D 0;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 for (i =3D -1; i <=3D 1; i++) {\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 temp =3D speed_ccb /=\n" + " (pixval+i);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_debug(\"DIU test p=\n" + "ixval i=3D%d, pixval=3D%lu, temp freq. =3D %u\\n\",\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=\n" + " =C2=A0 i, pixval, temp);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if ((temp < minpixcl=\n" + "ock) || (temp > maxpixclock))\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=\n" + " =C2=A0 pr_debug(\"DIU exceeds monitor range (%lu to %lu)\\n\",\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=\n" + " =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 minpixclock, maxpixclock);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 else if (abs(temp - =\n" + "pixclock) < err) {\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=\n" + " =C2=A0 pr_debug(\"Entered the else if block %d\\n\", i);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=\n" + " =C2=A0 err =3D abs(temp - pixclock);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=\n" + " =C2=A0 bestval =3D pixval + i;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=\n" + " =C2=A0 bestfreq =3D temp;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }\n" + "> > + =C2=A0 =C2=A0 =C2=A0 }\n" "> > +\n" - "> > + \302\240 \302\240 \302\240 pr_debug(\"DIU chose = %lx\\n\", bestval);\n" - "> > + \302\240 \302\240 \302\240 pr_debug(\"DIU error = %ld\\n NomPixClk \", err);\n" - "> > + \302\240 \302\240 \302\240 pr_debug(\"DIU: Best Freq = %lx\\n\", bestfreq);\n" - "> > + \302\240 \302\240 \302\240 /* Modify DIU_DIV in CCM SCFR1 */\n" - "> > + \302\240 \302\240 \302\240 temp = in_be32(ccm + CCM_SCFR1);\n" - "> \n" - "> Don't use offsets like + CCM_SCFR1. Create a structure and use that instead.\n" + "> > + =C2=A0 =C2=A0 =C2=A0 pr_debug(\"DIU chose =3D %lx\\n\", bestval);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 pr_debug(\"DIU error =3D %ld\\n NomPixClk \", err);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 pr_debug(\"DIU: Best Freq =3D %lx\\n\", bestfreq);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 /* Modify DIU_DIV in CCM SCFR1 */\n" + "> > + =C2=A0 =C2=A0 =C2=A0 temp =3D in_be32(ccm + CCM_SCFR1);\n" + ">=20\n" + "> Don't use offsets like + CCM_SCFR1. Create a structure and use that inst=\n" + "ead.\n" "\n" "Done in next patch version.\n" "\n" - "> > + \302\240 \302\240 \302\240 pr_debug(\"DIU: Current value of SCFR1: 0x%08x\\n\", temp);\n" - "> > + \302\240 \302\240 \302\240 temp &= ~DIU_DIV_MASK;\n" - "> > + \302\240 \302\240 \302\240 temp |= (bestval & DIU_DIV_MASK);\n" - "> > + \302\240 \302\240 \302\240 out_be32(ccm + CCM_SCFR1, temp);\n" - "> > + \302\240 \302\240 \302\240 pr_debug(\"DIU: Modified value of SCFR1: 0x%08x\\n\", temp);\n" - "> > + \302\240 \302\240 \302\240 iounmap(ccm);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 pr_debug(\"DIU: Current value of SCFR1: 0x%08x\\n\"=\n" + ", temp);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 temp &=3D ~DIU_DIV_MASK;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 temp |=3D (bestval & DIU_DIV_MASK);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 out_be32(ccm + CCM_SCFR1, temp);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 pr_debug(\"DIU: Modified value of SCFR1: 0x%08x\\n=\n" + "\", temp);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 iounmap(ccm);\n" "> > +}\n" "> > +\n" "> > +ssize_t mpc512x_show_monitor_port(int monitor_port, char *buf)\n" "> > +{\n" - "> > + \302\240 \302\240 \302\240 return snprintf(buf, PAGE_SIZE, \"0 - 5121 LCD\\n\");\n" - "> \n" + "> > + =C2=A0 =C2=A0 =C2=A0 return snprintf(buf, PAGE_SIZE, \"0 - 5121 LCD\\n\"=\n" + ");\n" + ">=20\n" "> There's no point in using snprintf since you're printing a string\n" "> literal. You can use sprintf.\n" "\n" @@ -263,42 +305,49 @@ "> > +\n" "> > +int mpc512x_set_sysfs_monitor_port(int val)\n" "> > +{\n" - "> > + \302\240 \302\240 \302\240 return 0;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 return 0;\n" "> > +}\n" "> > +\n" - "> > +static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_shared_fb;\n" + "> > +static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_s=\n" + "hared_fb;\n" "> > +\n" "> > +#if defined(CONFIG_FB_FSL_DIU) || \\\n" - "> > + \302\240 \302\240defined(CONFIG_FB_FSL_DIU_MODULE)\n" + "> > + =C2=A0 =C2=A0defined(CONFIG_FB_FSL_DIU_MODULE)\n" "> > +static inline void mpc512x_free_bootmem(struct page *page)\n" "> > +{\n" - "> > + \302\240 \302\240 \302\240 __ClearPageReserved(page);\n" - "> > + \302\240 \302\240 \302\240 BUG_ON(PageTail(page));\n" - "> > + \302\240 \302\240 \302\240 BUG_ON(atomic_read(&page->_count) > 1);\n" - "> > + \302\240 \302\240 \302\240 atomic_set(&page->_count, 1);\n" - "> > + \302\240 \302\240 \302\240 __free_page(page);\n" - "> > + \302\240 \302\240 \302\240 totalram_pages++;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 __ClearPageReserved(page);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 BUG_ON(PageTail(page));\n" + "> > + =C2=A0 =C2=A0 =C2=A0 BUG_ON(atomic_read(&page->_count) > 1);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 atomic_set(&page->_count, 1);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 __free_page(page);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 totalram_pages++;\n" "> > +}\n" "> > +\n" "> > +void mpc512x_release_bootmem(void)\n" "> > +{\n" - "> > + \302\240 \302\240 \302\240 unsigned long addr = diu_shared_fb.fb_phys & PAGE_MASK;\n" - "> > + \302\240 \302\240 \302\240 unsigned long size = diu_shared_fb.fb_len;\n" - "> > + \302\240 \302\240 \302\240 unsigned long start, end;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 unsigned long addr =3D diu_shared_fb.fb_phys & P=\n" + "AGE_MASK;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 unsigned long size =3D diu_shared_fb.fb_len;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 unsigned long start, end;\n" "> > +\n" - "> > + \302\240 \302\240 \302\240 if (diu_shared_fb.in_use) {\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 start = PFN_UP(addr);\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 end = PFN_DOWN(addr + size);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 if (diu_shared_fb.in_use) {\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 start =3D PFN_UP(add=\n" + "r);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 end =3D PFN_DOWN(add=\n" + "r + size);\n" "> > +\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 for (; start < end; start++)\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 mpc512x_free_bootmem(pfn_to_page(start));\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for (; start < end; =\n" + "start++)\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=\n" + " =C2=A0 mpc512x_free_bootmem(pfn_to_page(start));\n" "> > +\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 diu_shared_fb.in_use = false;\n" - "> > + \302\240 \302\240 \302\240 }\n" - "> > + \302\240 \302\240 \302\240 diu_ops.release_bootmem = NULL;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 diu_shared_fb.in_use=\n" + " =3D false;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 }\n" + "> > + =C2=A0 =C2=A0 =C2=A0 diu_ops.release_bootmem =3D NULL;\n" "> > +}\n" "> > +#endif\n" - "> \n" + ">=20\n" "> Do you really need to use reserve_bootmem? Have you tried kmalloc or\n" "> alloc_pages_exact()?\n" "\n" @@ -315,35 +364,38 @@ "> > + */\n" "> > +void __init mpc512x_init_diu(void)\n" "> > +{\n" - "> > + \302\240 \302\240 \302\240 struct device_node *np;\n" - "> > + \302\240 \302\240 \302\240 void __iomem *diu_reg;\n" - "> > + \302\240 \302\240 \302\240 phys_addr_t desc;\n" - "> > + \302\240 \302\240 \302\240 void __iomem *vaddr;\n" - "> > + \302\240 \302\240 \302\240 unsigned long mode, pix_fmt, res, bpp;\n" - "> > + \302\240 \302\240 \302\240 unsigned long dst;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 struct device_node *np;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 void __iomem *diu_reg;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 phys_addr_t desc;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 void __iomem *vaddr;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 unsigned long mode, pix_fmt, res, bpp;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 unsigned long dst;\n" "> > +\n" - "> > + \302\240 \302\240 \302\240 np = of_find_compatible_node(NULL, NULL, \"fsl,mpc5121-diu\");\n" - "> > + \302\240 \302\240 \302\240 if (!np) {\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 pr_err(\"No DIU node\\n\");\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 return;\n" - "> > + \302\240 \302\240 \302\240 }\n" - "> \n" + "> > + =C2=A0 =C2=A0 =C2=A0 np =3D of_find_compatible_node(NULL, NULL, \"fsl,=\n" + "mpc5121-diu\");\n" + "> > + =C2=A0 =C2=A0 =C2=A0 if (!np) {\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_err(\"No DIU node\\=\n" + "n\");\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 }\n" + ">=20\n" "> Shouldn't you be probing as an OF driver instead of manually searching\n" "> for the DIU node?\n" "\n" "No, not here.\n" "\n" "> > +\n" - "> > + \302\240 \302\240 \302\240 diu_reg = of_iomap(np, 0);\n" - "> > + \302\240 \302\240 \302\240 of_node_put(np);\n" - "> > + \302\240 \302\240 \302\240 if (!diu_reg) {\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 pr_err(\"Can't map DIU\\n\");\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 return;\n" - "> > + \302\240 \302\240 \302\240 }\n" + "> > + =C2=A0 =C2=A0 =C2=A0 diu_reg =3D of_iomap(np, 0);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 of_node_put(np);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 if (!diu_reg) {\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pr_err(\"Can't map DI=\n" + "U\\n\");\n" + "> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return;\n" + "> > + =C2=A0 =C2=A0 =C2=A0 }\n" "> > +\n" - "> > + \302\240 \302\240 \302\240 mode = in_be32(diu_reg + 0x1c);\n" - "> > + \302\240 \302\240 \302\240 if (mode != 1) {\n" - "> \n" + "> > + =C2=A0 =C2=A0 =C2=A0 mode =3D in_be32(diu_reg + 0x1c);\n" + "> > + =C2=A0 =C2=A0 =C2=A0 if (mode !=3D 1) {\n" + ">=20\n" "> How can in_be32() return a -1?\n" "\n" "It is a 1, not -1. I will use appropriate macro here and also\n" @@ -352,4 +404,4 @@ "Thanks for your review and comments!\n" Anatolij -4f3a1b159c607b297c5dc919d7b2be413b35d26b0f884a8e3bd732d1add09fe9 +3246a2dd964beb63ccee56539bd071cda968f5937c4485e9a2123d8827a426f1
diff --git a/a/1.txt b/N2/1.txt index bf28c25..19b6d17 100644 --- a/a/1.txt +++ b/N2/1.txt @@ -143,11 +143,13 @@ OK, will fix, thanks. > > + np = of_find_node_by_type(NULL, "cpu"); > > + if (np) { > > + unsigned int size; -> > + const unsigned int *prop > > + of_get_property(np, "bus-frequency", &size); +> > + const unsigned int *prop = +> > + of_get_property(np, "bus-frequency", &size); > > Since you don't use 'size', you can skip it: > -> const unsigned int *prop > of_get_property(np, "bus-frequency", NULL); +> const unsigned int *prop = +> of_get_property(np, "bus-frequency", NULL); > > > + } else { > > + pr_err("Can't find \"cpu\" node.\n"); @@ -335,3 +337,7 @@ change to use a struct instead of adding offset to register base. Thanks for your review and comments! Anatolij +_______________________________________________ +Linuxppc-dev mailing list +Linuxppc-dev@lists.ozlabs.org +https://lists.ozlabs.org/listinfo/linuxppc-dev diff --git a/a/content_digest b/N2/content_digest index 1eb84f7..ecaa0b1 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -3,7 +3,7 @@ "ref\0s2ted82fe3e1004291905mf562f0cbi24054bb973aa2dbb@mail.gmail.com\0" "From\0Anatolij Gustschin <agust@denx.de>\0" "Subject\0Re: [PATCH 3/5] powerpc/mpc5121: shared DIU framebuffer support\0" - "Date\0Fri, 30 Apr 2010 10:19:47 +0000\0" + "Date\0Fri, 30 Apr 2010 12:19:47 +0200\0" "To\0Timur Tabi <timur.tabi@gmail.com>\0" "Cc\0linux-fbdev@vger.kernel.org" wd@denx.de @@ -159,11 +159,13 @@ "> > + \302\240 \302\240 \302\240 np = of_find_node_by_type(NULL, \"cpu\");\n" "> > + \302\240 \302\240 \302\240 if (np) {\n" "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 unsigned int size;\n" - "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 const unsigned int *prop > > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 of_get_property(np, \"bus-frequency\", &size);\n" + "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 const unsigned int *prop =\n" + "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 of_get_property(np, \"bus-frequency\", &size);\n" "> \n" "> Since you don't use 'size', you can skip it:\n" "> \n" - "> const unsigned int *prop > of_get_property(np, \"bus-frequency\", NULL);\n" + "> const unsigned int *prop =\n" + "> of_get_property(np, \"bus-frequency\", NULL);\n" "> \n" "> > + \302\240 \302\240 \302\240 } else {\n" "> > + \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 pr_err(\"Can't find \\\"cpu\\\" node.\\n\");\n" @@ -350,6 +352,10 @@ "change to use a struct instead of adding offset to register base.\n" "\n" "Thanks for your review and comments!\n" - Anatolij + "Anatolij\n" + "_______________________________________________\n" + "Linuxppc-dev mailing list\n" + "Linuxppc-dev@lists.ozlabs.org\n" + https://lists.ozlabs.org/listinfo/linuxppc-dev -4f3a1b159c607b297c5dc919d7b2be413b35d26b0f884a8e3bd732d1add09fe9 +6778f5d3f902333e9f9ae6a3fac0ef32a91918de85cf1a1587e744e4eeddcc33
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.