* Re: arch/powerpc for lite5200b
From: Domen Puncer @ 2007-04-25 5:29 UTC (permalink / raw)
To: Pradyumna Sampath; +Cc: linuxppc-embedded
In-Reply-To: <f87351060704240431m2218910cs65510472218459b0@mail.gmail.com>
On 24/04/07 17:01 +0530, Pradyumna Sampath wrote:
> Hi Everyone,
>
> I have been trying to get the linux-2.6.20-rt3 for arch/powerpc to
> boot on my lite5200b. Here are the things that I have tried so far. I
> have a custom board based on the lite5200b and then the evaluation
> board too.
>
> - I have a working 2.6.16-rt29 kernel compiled using ARCH=ppc for both
> these boards.
>
> - I tried the default configs for lite5200 for both the custom board
> and the EVB. Both hang at "Transferring control to Linux (at address
> 00000000) ..."
Device tree missing?
You need to compile it with dtc, then in u-boot use something like:
bootp; tftp 0x400000 uImage.ppc; tftp 0x600000 lite5200b.dtb
set your bootargs
bootm 0x400000 - 0x600000
>
> - I downloaded roberts
> http://www.pengutronix.de/oselas/bsp/phytec/index_en.html#phyCORE-MPC5200B-tiny
> patches, but still ended up with the same result.
>
> I read ( http://www.nabble.com/Re%3A-MPC52xx-support-rework-for-2.6.20-p7568655.html
> ) that the arch/powerpc port for lite5200 works right out of the box.
> Am I doing something wrong ?
>
> The one thing I am yet to try is to do a git checkout from
> git.secretlabs.ca. Unfortunately thanks to some internal firewalling
> policies I still have not been able to do that.
kernel.org tree will also work with lite5200b, if you need ethernet
download fec/bestcomm patches from Grant's tree.
Domen
^ permalink raw reply
* Re: [PATCH] 64K page support for kexec
From: Milton Miller @ 2007-04-25 5:48 UTC (permalink / raw)
To: Olof Johansson, Luke Browning, Benjamin Herrenschmidt; +Cc: ppcdev, cbe-oss-dev
In-Reply-To: <20070424230708.GA10401@lixom.net>
>
>
> > > > + /*
> > > > + * FIXME, this could be made more efficient by storing the type
> > > > + * of hash algorithm in mmu_psize_defs[]. The code below
> assumes
> > > > + * the number of bits in the va representing the offset in the
> > > > + * page is less than 23. This affects the hash algorithm that
> is
> > > > + * used. When 16G pages are supported, a new hash algorithm
> > > > + * needs to be provided. See POWER ISA Book III.
> > > > + *
> > > > + * The code below works for 16M, 64K, and 4K pages.
> > > > + */
> > >
> > > A BUG_ON() when other sizes are hit could be a good idea?
> >
> > a BUG_ON if the B bit is set would be useful too. (that is 1T segment
> > HPTE).
>
> Yep.
NNNNOOOOO!!!!
Do NOT add any BUG() is this code!
Look at the context:
(1) We are tearing down *ALL* mappings. That includes the
kernel linear mapping and the mapping of the kernel text.
(2) We are in real mode. There is no way back to virtural
mode. See (1).
(3) We hove put the new kernel in memory. There is no data
that was not a part of the static data or bss sections. There
are no per-cpu variables. Nothing with vmalloc. Nothing with
kmalloc. Nothing with alloc_pages.
(4) If this is the panic kernel case, we are allready crashed
and trying to get into a new envrionment to dump memory.
Intentionally failing that is the last thing you want to do.
(5) This hook is only used by kexec_sequence, called by
machine_kexec. At the beginning of that function there
is the comment "its too late to fail here."
Bottom line: Don't even think of causing any trap here, just
execute as best as you know how. You dohn't have any way to
tell the user you failed anyways.
milton
^ permalink raw reply
* Re: [PATCH] generic check_legacy_ioport
From: Arnd Bergmann @ 2007-04-25 7:49 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Christian Krafft, Olaf Hering, linuxppc-dev
In-Reply-To: <0e76c9675bd1bcf4b7ed6bf83da8c3d7@kernel.crashing.org>
On Wednesday 25 April 2007, Segher Boessenkool wrote:
> > I don't think there are any powerpc machines where it can find=20
> > something
> > there, but we decided to leave the code architecture independent in
> > case there ever are, and just to add the check_legacy_ioport call in=20
> > there.
>=20
> SLOF/JS21 (at least some versions of it) have an "ipmi"
> node on the "isa" bus. =A0And the kernel ipmi driver actually
> works on it, too (no idea about the currently proposed
> scanning though -- but the principle is correct at least).
It won'd be found by the of_platform_driver part of ipmi,
since devices on the ISA bus do not get added to the linux
device tree.
If the ipmi node on the ISA bus is located at the standardized
legacy I/O port range, it should get found by the later probing,
if check_legacy_ioport allows it.
We might still want to have the driver use a proper of_device,
which would require the maple platform code to add this device
during probing.
Arnd <><
^ permalink raw reply
* Re: [PATCH 1/5] [PPC] Rework Kconfig dependancies for Xilinx Virtex ppc405 platform
From: Peter Korsgaard @ 2007-04-25 12:03 UTC (permalink / raw)
To: linuxppc-embedded
In-Reply-To: <11766220692537-git-send-email-grant.likely@secretlab.ca>
>>>>> "GL" == Grant Likely <grant.likely@secretlab.ca> writes:
GL> Reverse dependency order for Xilinx Virtex parts. For these
GL> parts, It makes more sense for boards/chips to specify which
GL> features they provide instead of the features listing the parts
GL> they are implemented in. I think it also makes adding new board
GL> ports simpler.
GL> Signed-off-by: Grant Likely <grant.likely@secretlab.ca> ---
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
--
Bye, Peter Korsgaard
^ permalink raw reply
* Re: [PATCH 2/5] [PPC] Merge common virtex header files
From: Peter Korsgaard @ 2007-04-25 12:07 UTC (permalink / raw)
To: linuxppc-embedded
In-Reply-To: <11766220693636-git-send-email-grant.likely@secretlab.ca>
>>>>> "GL" == Grant Likely <grant.likely@secretlab.ca> writes:
Hi,
GL> The header files for the ml403 and ml300 are virtually identical,
GL> merge them into a single file.
GL> diff --git a/include/asm-ppc/ibm4xx.h b/include/asm-ppc/ibm4xx.h
GL> index 92fd02d..ed6891a 100644
GL> --- a/include/asm-ppc/ibm4xx.h
GL> +++ b/include/asm-ppc/ibm4xx.h
GL> @@ -47,12 +47,8 @@
GL> #include <platforms/4xx/walnut.h>
GL> #endif
GL>
GL> -#if defined(CONFIG_XILINX_ML300)
GL> -#include <platforms/4xx/xilinx_ml300.h>
GL> -#endif
GL> -
GL> -#if defined(CONFIG_XILINX_ML403)
GL> -#include <platforms/4xx/xilinx_ml403.h>
GL> +#if defined(CONFIG_XILINX_VIRTEX)
GL> +#include <platforms/4xx/virtex.h>
GL> #endif
Could we make that
#if defined(CONFIG_XILINX_ML300) || defined(CONFIG_XILINX_ML403)
instead? I'm using RedBoot on a virtex platform, and hence another
struct bd_info than the ML300/ML403.
Other than that,
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
--
Bye, Peter Korsgaard
^ permalink raw reply
* Re: [PATCH 3/5] [PPC] New registration for common Xilinx Virtex ppc405 platform devices
From: Peter Korsgaard @ 2007-04-25 12:11 UTC (permalink / raw)
To: linuxppc-embedded
In-Reply-To: <1176622070387-git-send-email-grant.likely@secretlab.ca>
>>>>> "GL" == Grant Likely <grant.likely@secretlab.ca> writes:
Hi,
GL> Currently virtex support in mainline make use of the
GL> infrastructure in arch/ppc/syslib/ppc_sys.c for registering common
GL> devices on virtex ppc405 platforms. The ppc_sys.c code is not
GL> well suited to the dynamic nature of FPGA designs and makes adding
GL> new board ports more complex. This patch adds a new listing of
GL> common devices which does not depend on the ppc_sys.c
GL> infrastructure.
GL> +/* UART 8250 driver platform data table */
GL> +struct plat_serial8250_port virtex_serial_platform_data[] = {
GL> +#if defined(XPAR_UARTNS550_0_BASEADDR)
GL> + XPAR_UART(0),
GL> +#endif
GL> +#if defined(XPAR_UARTNS550_1_BASEADDR)
GL> + XPAR_UART(1),
GL> +#endif
GL> +#if defined(XPAR_UARTNS550_2_BASEADDR)
GL> + XPAR_UART(2),
GL> +#endif
GL> +#if defined(XPAR_UARTNS550_3_BASEADDR)
GL> + XPAR_UART(3),
GL> +#endif
GL> + { }, /* terminated by empty record */
GL> +};
Could we at the same time increase the amount of UARTs supported? We
have designs with 5 16550s.
Other than that,
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
--
Bye, Peter Korsgaard
^ permalink raw reply
* Re: [PATCH 4/5] [PPC] Stop using ppc_sys for Xilinx Virtex boards
From: Peter Korsgaard @ 2007-04-25 12:12 UTC (permalink / raw)
To: linuxppc-embedded
In-Reply-To: <11766220702647-git-send-email-grant.likely@secretlab.ca>
>>>>> "GL" == Grant Likely <grant.likely@secretlab.ca> writes:
Hi,
GL> The arch/ppc/syslib/ppc_sys.c infrastructure does not work well
GL> for the virtex ports. Move the ml300 and ml403 board ports over
GL> to use the new virtex_devices infrastructure.
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
--
Bye, Peter Korsgaard
^ permalink raw reply
* Re: [PATCH 5/5] [PPC] Add uartlite boot console driver for the zImage wrapper
From: Peter Korsgaard @ 2007-04-25 12:13 UTC (permalink / raw)
To: linuxppc-embedded
In-Reply-To: <11766220713124-git-send-email-grant.likely@secretlab.ca>
>>>>> "GL" == Grant Likely <grant.likely@secretlab.ca> writes:
Hi,
GL> Signed-off-by: Grant Likely <grant.likely@secretlab.ca> ---
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
--
Bye, Peter Korsgaard
^ permalink raw reply
* Re: Flat device tree definitions for FLASH and MTD partitioning
From: Wolfgang Grandegger @ 2007-04-25 12:55 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-embedded
In-Reply-To: <4618DE4A.4020404@ru.mvista.com>
Sergei Shtylyov wrote:
> Hello, I wrote:
>
>>> are there already some thought or even example code on how to define
>>> Flash memory and MTD partitions in the DTS. This would avoid the
>>> ackward MTD partitioning via static structure or boot line argument
>>> and could be done in a generic way.
>
>> See Documentation/powerpc/booting-without-of.txt and
>> drivers/mtd/physmap_of.c for what's been already done.
>
> I meant drivers/mtd/maps/physmap_of.c
I had a closer look to this driver. Unfortunately, it can not handle yet
multiple FLASH banks. This could be achieved with an additional
property "bank_count". What do you think?
Wolfgang.
^ permalink raw reply
* Re: [PATCH] Xilinx framebuffer device driver
From: Andrei Konovalov @ 2007-04-25 13:06 UTC (permalink / raw)
To: Grant Likely; +Cc: Rick Moleres, linuxppc-embedded
In-Reply-To: <528646bc0704241517g595bf7bfqe54e40b05f74e933@mail.gmail.com>
Grant Likely wrote:
> On 4/24/07, Andrei Konovalov <akonovalov@ru.mvista.com> wrote:
>> Add support for the video controller IP block included into Xilinx
>> ML300 and
>> ML403 reference designs.
>>
>> Signed-off-by: Andrei Konovalov <akonovalov@ru.mvista.com>
>> ---
>>
>> This patch relies on the "Patchset to establish sanity in Xilinx
>> Virtex support" by Gran Likely to have
>> the frame buffer device registered on the platform bus. Without this
>> patchset one needs to fill in
>> the struct platform_device and make sure platform_device_register() is
>> called elsewhere.
>>
>> Reviews and comments are welcome.
>>
>> Would be nice to get this driver into mainline for the 2.6.22.
>
> Quick comment on first perusal: The driver uses the out_be32 macro
> directly for accessing registers, which doesn't work if the FB block
> is configured for DCR access (like the ML403 reference design).
Yes, that's true.
But (at least) the EDK 8.1 reference design for ML403 has also opb2dcr bridge.
That's why I've tested the patch without problems on ML403 (in addition
to ML300).
I'll add DCR access, but not sure if I need a separate bitstream to
test DCR access (could I access the DCR registers directly in presence
of the bridge or not).
> There will need to be a property in the platform device binding to determine
> how to access registers.
OK.
The only problem is that there is no indication in the xparameters.h of how the
registers should be accessed (via DCR or opb).
Let's suppose XPAR_TFT_0_USE_DCR would be added by EDK (like XPAR_XINTC_USE_DCR
is used for the interrupt controller).
> Cheers,
> g.
>
Thanks,
Andrei
^ permalink raw reply
* Re: [PATCH] 64K page support for kexec
From: Luke Browning @ 2007-04-25 13:06 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Paul Mackerras, cbe-oss-dev, Arnd Bergmann
In-Reply-To: <1177454894.14873.142.camel@localhost.localdomain>
On Wed, 2007-04-25 at 08:48 +1000, Benjamin Herrenschmidt wrote:
> Getting better :-)
>
> Sorry for the constant nagging, let's say I'm a bit perfectionist...
>
so am I. That is why I redid the patch instead of going with the
previous version.
> > -static unsigned long slot2va(unsigned long hpte_v, unsigned long slot)
> > -{
> > - unsigned long avpn = HPTE_V_AVPN_VAL(hpte_v);
> > - unsigned long va;
> > -
> > - va = avpn << 23;
> > -
> > - if (! (hpte_v & HPTE_V_LARGE)) {
> > - unsigned long vpi, pteg;
> > -
> > - pteg = slot / HPTES_PER_GROUP;
> > - if (hpte_v & HPTE_V_SECONDARY)
> > - pteg = ~pteg;
>
> Hrm... hpte_decode ends up being a pretty big function... I suppose
> that's ok.
>
It is bigger because it combines two functions. I think it is better
this way as it will make it easier to add support for 16G pages and 1T
segments in the future.
> > +#define LP_SHIFT 12
> > +#define LP_BITS 8
> > +#define LP_MASK(i) ((((1 << LP_BITS) - 1) >> (i)) << LP_SHIFT)
> > +
> > +static void hpte_decode(hpte_t *hpte, unsigned long slot,
> > + int *psize, unsigned long *va)
> > +{
> > + unsigned long hpte_r = hpte->r;
> > + unsigned long hpte_v = hpte->v;
> > + unsigned long avpn;
> > + int i, size, shift, penc, avpnm_bits;
> > +
> > + if (!(hpte_v & HPTE_V_LARGE))
> > + size = MMU_PAGE_4K;
> > +#if 0
> > + else if (hpte_v & 0x4000000000000000UL)
> > + size = MMU_PAGE_16G;
> > +#endif
>
> Remove the above. I don't think it's right anyway. We'll deal with 16G
> pages when we start using them.
OK. I put it there mainly as a place holder. How many page sizes do
you expect to support in a 1TB segment?
>
> > + else if (!(hpte_r & LP_MASK(0)))
> > + size = MMU_PAGE_16M;
>
> Is that correct ? (The above).
> I haven't quite noticed in the previous
> instances of the patch, sorry about that but I don't think that's the
> way to detect the "old style" 16M pages... I -suspect- that the normal
> algorithm will work for them, that is, they'll have a penc of 0 which
> will match what's in the mmu_psize_defs[MMU_PAGE_16M] but it's worth
> actually testing it.
Ok.
>
> Now that I think about it, it's possible that I lead you on the wrong
> track there initially... Sorry about that.
>
> I think your above code will treat anything with a penc of 0 as a 16M
> page, which might be true with current implementations, is also, I
> think, not mandated by the arch, is it ? (I don't have my 2.03 at hand
> as I'm writing this email).
right. The encoding is implementation dependent. There is a minimum
size requirement that comes into play, but that works the other way.
penc of zero has a minimum page size requirement of 8K.
I will remove the test and use the array for the comparison.
>
> > + else {
> > + for (i = 0; i < LP_BITS; i++) {
> > + if ((hpte_r & LP_MASK(i+1)) == LP_MASK(i+1))
> > + break;
> > + }
> > + penc = LP_MASK(i+1) >> LP_SHIFT;
> > + for (size = MMU_PAGE_64K; size < MMU_PAGE_16M; size++) {
> > + if (!mmu_psize_defs[size].shift)
> > + continue;
> > + if (penc == mmu_psize_defs[size].penc)
> > + break;
> > + }
> > + }
> >
> > - vpi = ((va >> 28) ^ pteg) & htab_hash_mask;
> > + /*
> > + * FIXME, this could be made more efficient by storing the type
> > + * of hash algorithm in mmu_psize_defs[]. The code below assumes
> > + * the number of bits in the va representing the offset in the
> > + * page is less than 23. This affects the hash algorithm that is
> > + * used. When 16G pages are supported, a new hash algorithm
> > + * needs to be provided. See POWER ISA Book III.
> > + *
> > + * The code below works for 16M, 64K, and 4K pages.
> > + */
>
> I'm not 100% certain about your comment. The by type of hash algorithm
> you mean the segment size right ? This is not directly related to the
> page size. While 1T segments are mandatory for 16G pages, they can also
> hold normal page sizes... If we're going to implement support for 1T
> segment, we should get the segment size (and thus the hash algorithm)
> from the B bit of the PTE.
yes. what you said is correct. I was thinking that you might want to
double the number of array elements so that you could apply a different
hash algorithm based on the type of segments.
>
> In fact, when doing 1T segments, we'll have to deal with them regardless
> of the page size (the hashing will be different for all page sizes).
>
> > + shift = mmu_psize_defs[size].shift;
> > + if (mmu_psize_defs[size].avpnm)
> > + avpnm_bits = __ilog2_u64(mmu_psize_defs[size].avpnm) + 1;
> > + else
> > + avpnm_bits = 0;
> > + if (shift - avpnm_bits <= 23) {
> > + avpn = HPTE_V_AVPN_VAL(hpte_v) << 23;
> > +
> > + if (shift < 23) {
> > + unsigned long vpi, pteg;
> > +
> > + pteg = slot / HPTES_PER_GROUP;
> > + if (hpte_v & HPTE_V_SECONDARY)
> > + pteg = ~pteg;
> > + vpi = ((avpn >> 28) ^ pteg) & htab_hash_mask;
> > + avpn |= (vpi << mmu_psize_defs[size].shift);
> > + }
> > + }
> > +#if 0
> > + /* 16GB page hash, p > 23 */
> > + else {
> >
> > - va |= vpi << PAGE_SHIFT;
> > }
> > +#endif
>
> Just don't keep the code in #if 0, just a comment about something
> needing to be done for 16G ...
>
ok.
> > - return va;
> > + *va = avpn;
> > + *psize = size;
> > }
> >
> > /*
> > @@ -374,8 +420,6 @@ static unsigned long slot2va(unsigned lo
> > *
> > * TODO: add batching support when enabled. remember, no dynamic memory here,
> > * athough there is the control page available...
> > - *
> > - * XXX FIXME: 4k only for now !
> > */
> > static void native_hpte_clear(void)
> > {
> > @@ -383,6 +427,7 @@ static void native_hpte_clear(void)
> > hpte_t *hptep = htab_address;
> > unsigned long hpte_v;
> > unsigned long pteg_count;
> > + int psize;
> >
> > pteg_count = htab_hash_mask + 1;
> >
> > @@ -408,8 +453,9 @@ static void native_hpte_clear(void)
> > * already hold the native_tlbie_lock.
> > */
> > if (hpte_v & HPTE_V_VALID) {
> > + hpte_decode(hptep, slot, &psize, &hpte_v);
> > hptep->v = 0;
> > - __tlbie(slot2va(hpte_v, slot), MMU_PAGE_4K);
> > + __tlbie(hpte_v, psize);
> > }
> > }
> >
> >
>
^ permalink raw reply
* Re: [PATCH 1/6] Start split out of common open firmware code
From: Segher Boessenkool @ 2007-04-25 13:07 UTC (permalink / raw)
To: David Miller; +Cc: sfr, paulus, linuxppc-dev
In-Reply-To: <20070424.212736.115930755.davem@davemloft.net>
>> The SPARC tree carries the same broken code -- is
>> this just a case of code copying, or are there
>> actual SPARC machines with such broken trees?
>
> There have been many cases of missed or even erroneous
> properties on sparc tree, but I am not too sure about
> the cell counting cases.
>
> Let's do this, once the consolidation is done keeping
> behavior as-is, give me a patch to review that changing
> the cell counting bits and I'll verify it against
> all the machines I have here plus some OFW tree dumps.
Okay, good plan. Let's try this as soon as the current
patch series hits the powerpc tree.
Segher
^ permalink raw reply
* Re: [PATCH 1/6] Start split out of common open firmware code
From: Segher Boessenkool @ 2007-04-25 13:11 UTC (permalink / raw)
To: David Miller; +Cc: sfr, paulus, linuxppc-dev
In-Reply-To: <20070424.212902.00454541.davem@davemloft.net>
>> end up with the OF analogue to an MSI layer that only
>> ever can work on intel compatible platforms.
>
> Please don't use this straw-man.
>
> These days it works perfectly fine on sparc64.
Yes, but how long did that take? And why? That's my
point, not that "after this things cannot be solved
anymore" -- sure they can, it gets a lot harder though.
Segher
^ permalink raw reply
* Re: [PATCH 3/6] Consolidate of_find_property
From: Segher Boessenkool @ 2007-04-25 13:13 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: ppc-dev, Paul Mackerras, David S. Miller, Stephen Rothwell
In-Reply-To: <1177477630.14873.185.camel@localhost.localdomain>
>> The question is, what ppc drivers would break if we used strcmp
>> instead of strcasecmp? I have a dim memory that some Apple machines
>> had "ata" and others had "ATA" for the hard disk, for instance.
>
> ata/ATA and ide/IDE is the main one that comes to mind, but I wouldn't
> exclude something like Powermac vs. PowerMac or that sort of thing... I
> remember a case related bug about 2 years ago but can't find what it
> was ...
>
> I think it's safe to settle on strcasecmp for most things for now
Yes, it's hard to imagine any case where strcasecmp(),
although technically incorrect, would break anything.
Please document in the code that is _is_ wrong and _why_
it is done though.
Segher
^ permalink raw reply
* Re: [PATCH] generic check_legacy_ioport
From: Segher Boessenkool @ 2007-04-25 13:33 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Christian Krafft, Olaf Hering, linuxppc-dev
In-Reply-To: <200704250949.17795.arnd@arndb.de>
>> SLOF/JS21 (at least some versions of it) have an "ipmi"
>> node on the "isa" bus. =A0And the kernel ipmi driver actually
>> works on it, too (no idea about the currently proposed
>> scanning though -- but the principle is correct at least).
>
> It won'd be found by the of_platform_driver part of ipmi,
> since devices on the ISA bus do not get added to the linux
> device tree.
Drat. Why not?
> If the ipmi node on the ISA bus is located at the standardized
> legacy I/O port range, it should get found by the later probing,
> if check_legacy_ioport allows it.
"Standardised" hahaha. No, it is at some other
address.
> We might still want to have the driver use a proper of_device,
> which would require the maple platform code to add this device
> during probing.
That sounds like the only option then. Does
this have to be done per platform though? It
sounds perfectly safe to do it in more generic
code.
Segher
^ permalink raw reply
* Re: Flat device tree definitions for FLASH and MTD partitioning
From: Sergei Shtylyov @ 2007-04-25 13:35 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: linuxppc-embedded
In-Reply-To: <462F4FD1.6080900@grandegger.com>
Hello.
Wolfgang Grandegger wrote:
>>>> are there already some thought or even example code on how to define
>>>> Flash memory and MTD partitions in the DTS. This would avoid the
>>>> ackward MTD partitioning via static structure or boot line argument
>>>> and could be done in a generic way.
>>> See Documentation/powerpc/booting-without-of.txt and
>>> drivers/mtd/physmap_of.c for what's been already done.
>> I meant drivers/mtd/maps/physmap_of.c
> I had a closer look to this driver. Unfortunately, it can not handle yet
> multiple FLASH banks. This could be achieved with an additional
> property "bank_count". What do you think?
I'm not sure what banks you're talking about -- note that this driver is equivalen in functionality to the plain 'physmap' driver and there's inherited "bank-width" property that handles flash banking.
> Wolfgang.
MBR, Sergei
^ permalink raw reply
* Re: Flat device tree definitions for FLASH and MTD partitioning
From: Wolfgang Grandegger @ 2007-04-25 14:02 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-embedded
In-Reply-To: <462F592F.7000409@ru.mvista.com>
Sergei Shtylyov wrote:
> Hello.
>
> Wolfgang Grandegger wrote:
>
>>>>> are there already some thought or even example code on how to
>>>>> define Flash memory and MTD partitions in the DTS. This would avoid
>>>>> the ackward MTD partitioning via static structure or boot line
>>>>> argument and could be done in a generic way.
>
>>>> See Documentation/powerpc/booting-without-of.txt and
>>>> drivers/mtd/physmap_of.c for what's been already done.
>
>>> I meant drivers/mtd/maps/physmap_of.c
>
>> I had a closer look to this driver. Unfortunately, it can not handle
>> yet multiple FLASH banks. This could be achieved with an additional
>> property "bank_count". What do you think?
>
> I'm not sure what banks you're talking about -- note that this driver
> is equivalen in functionality to the plain 'physmap' driver and there's
> inherited "bank-width" property that handles flash banking.
Yes, I know. Nevertheless the driver cannot handle flash regions
composed of more than on FLASH bank because we need to probe every bank.
Search for "BANK" in drivers/mtd/maps" to understand what I mean.
Wolfgang.
^ permalink raw reply
* Re: Flat device tree definitions for FLASH and MTD partitioning
From: Sergei Shtylyov @ 2007-04-25 14:34 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: linuxppc-embedded
In-Reply-To: <462F5F86.3060309@grandegger.com>
Hello.
Wolfgang Grandegger wrote:
>>>>>> are there already some thought or even example code on how to
>>>>>> define Flash memory and MTD partitions in the DTS. This would
>>>>>> avoid the ackward MTD partitioning via static structure or boot
>>>>>> line argument and could be done in a generic way.
>>>>> See Documentation/powerpc/booting-without-of.txt and
>>>>> drivers/mtd/physmap_of.c for what's been already done.
>>>> I meant drivers/mtd/maps/physmap_of.c
>>> I had a closer look to this driver. Unfortunately, it can not handle
>>> yet multiple FLASH banks. This could be achieved with an additional
>>> property "bank_count". What do you think?
Please use hyphen, not underscore in property names ("device_type" seems an only exception from this rule).
>> I'm not sure what banks you're talking about -- note that this driver
>> is equivalen in functionality to the plain 'physmap' driver and
>> there's inherited "bank-width" property that handles flash banking.
> Yes, I know. Nevertheless the driver cannot handle flash regions
> composed of more than on FLASH bank because we need to probe every bank.
> Search for "BANK" in drivers/mtd/maps" to understand what I mean.
Hm, from what I could see, those banks are separate maps registered to MTD subsys separately.
What's the point of adding a count property when we simply can define multiple "rom" devices?
> Wolfgang.
MBR, Sergei
^ permalink raw reply
* Re: [PATCH][RFC] i2c: adds support for i2c bus on 8xx
From: Vitaly Bordug @ 2007-04-25 17:06 UTC (permalink / raw)
To: Jean Delvare; +Cc: linuxppc-dev@ozlabs.org, lkml
In-Reply-To: <20070423111950.129856eb@hyperion.delvare>
[-- Attachment #1: Type: text/plain, Size: 4942 bytes --]
Jean Delvare wrote:
> Hi Vitaly,
>
> On Sun, 22 Apr 2007 15:29:37 +0400, Vitaly Bordug wrote:
>
>> On Sat, 21 Apr 2007 09:57:07 +0200 Jean Delvare wrote:
>>
>>> I wonder what's the point of having a separate i2c algorithm driver.
>>> We don't expect any other driver than i2c-rpx to ever use it, do we?
>>> In that case, all the code should be added to i2c-rpx directly, this
>>> will makes things more simple and more efficient.
>>>
>> That is how it was back in 2.4 - if you see combine is a good move,
>> I'm OK with it. But what shouldn't be rpc then - basically rpx(lite) is
>> 8xx-based target, so let's call it all mpc8xx then.
>>
>
> Sure, I'm fine with a name change. If it makes more sense to name that
> driver i2c-mpc8xx, that's OK with me.
>
>
>>>> + tmo = jiffies + 1 * HZ;
>>>> + while (!(in_8(&i2c->i2c_i2cer) & 0x11 || time_after(jiffies, tmo))) ;/* Busy wait, with a timeout */
>>>>
>>> This could result in a one-second busy loop, not very friendly for
>>> other drivers. It should sleep while waiting. Line too long, please
>>> fold.
>>>
>> Can you please elaborate a little here (or just point to the
>> similar code)? I assume we should not block here, handling timeout
>> in a waitqueue...
>>
>
> Blocking is not a problem. The problem is that you are keeping the CPU
> for yourself while waiting, for up to one full second. That's not
> acceptable. You should at least call schedule() or cond_resced() (I
> don't know the difference, I admit) and/or cpu_relax() as is done in
> i2c-mpc, i2c-ibm_iic and scx200_acb, or even sleep, as is done in
> i2c-omap. Search for "time_after" in these 4 drivers for examples. I
> believe that sleeping is more friendly.
>
>
OK, very clear, thanks.
>>> You do not appear to handle repeated start. I can tell because the
>>> code handles all messages the exact same way, be they the first,
>>> second or last message of a group. This means that you don't really
>>> implement the I2C protocol, but an approximation of it. It might be
>>> sufficient for some I2C chips, but others will break. Look in the
>>> specifications of your device for how this could be fixed.
>>>
>> I doubt 8xx has a full-fledged i2c protocol stuff onboard, and basic
>> code that were residing in 2.4 repo suite my needs quite well (afaict
>> many others just don't care :)).
>> I just think it is silly to drop the code already implemented and working
>> even if it requires some efforts to bring it up to shape.
>>
>
> Well as far as I can see, only the repeated start is missing, so it's
> not that far from a complete implementation. If the hardware can do it,
> you simply have to add it to the driver. If the hardware really doesn't
> do it (which would surprise me, but you never know), of course you
> cannot implement it in the driver and we'll have to live with (well,
> without) it. But that's definitely an issue to keep in mind if I2C chip
> drivers start failing when used together with this bus driver.
>
>
>>>> +static struct i2c_adapter rpx_ops = {
>>>>
>>> Could be const?
>>>
>>>
>> prolly yes.
>>
>>>> + .owner = THIS_MODULE,
>>>> + .name = "m8xx",
>>>>
>>> Find a better name (e.g. "i2c-rpx").
>>>
>>>
>> What about mpc8xx?
>>
>
> i2c-mpc8xx then, OK.
>
>
>>>> +/* Structure for a device driver */
>>>> +static struct device_driver i2c_rpx_driver = {
>>>> + .name = "fsl-i2c-cpm",
>>>> + .bus = &platform_bus_type,
>>>> + .probe = i2c_rpx_probe,
>>>> + .remove = i2c_rpx_remove,
>>>> +};
>>>>
>>> Why don't you declare it as a struct platform_driver, register it with
>>> platform_driver_register() and unregister it with
>>> platform_driver_unregister()?
>>>
>> Well. This stuff belongs to CPM1, of the mpc8xx family, but the
>> target boards are different, and they may/should provide board
>> specific inits and filling of platform data. With
>> platform_driver_register we may end up with ifdef stuff here
>> (which is evil).
>>
>
> I don't follow you here, sorry. Platform devices are declared by
> board-specific code which can include all the needed initialization.
> And device-specific data can be carried to the platform driver for
> further use. The platform device/driver infrastructure is meant to
> handle that kind of situation, so there really is no excuse that I can
> see not to use it. i2c-omap and i2c-mpc use it. As a matter of fact you
> _are_ declaring a platform driver (.bus = &platform_bus_type), just not
> using the standard way.
>
>
Standard way here - platform devices got registered from elsewhere -
from arch/ppc/ppc_sys.c if arch/ppc or from
arch/powerpc/sysdev/fsl_soc.c if powerpc.
Every way (powerpc is more flexible since is pulling the information
from the firmware-passed device tree) fills in the resources and
platform data, and
is capable with device/drive bound you are talking about.
--
Thanks, Vitaly
[-- Attachment #2: Type: text/html, Size: 6359 bytes --]
^ permalink raw reply
* Re: [PATCH][RFC][POWERPC] i2c: adds support for i2c bus on 8xx
From: Vitaly Bordug @ 2007-04-25 17:01 UTC (permalink / raw)
To: Olof Johansson; +Cc: Jean Delvare, linuxppc-dev@ozlabs.org, lkml
In-Reply-To: <20070423025709.GA28805@lixom.net>
[-- Attachment #1: Type: text/plain, Size: 4624 bytes --]
Olof Johansson wrote:
> On Fri, Apr 20, 2007 at 08:27:14AM +0400, Vitaly Bordug wrote:
>
>
>> diff --git a/arch/powerpc/platforms/8xx/mpc885ads_setup.c b/arch/powerpc/platforms/8xx/mpc885ads_setup.c
>> index 9bd81c7..d32e066 100644
>> --- a/arch/powerpc/platforms/8xx/mpc885ads_setup.c
>> +++ b/arch/powerpc/platforms/8xx/mpc885ads_setup.c
>> @@ -51,6 +51,7 @@ static void init_smc1_uart_ioports(struc
>> static void init_smc2_uart_ioports(struct fs_uart_platform_info* fpi);
>> static void init_scc3_ioports(struct fs_platform_info* ptr);
>> static void init_irda_ioports(void);
>> +static void init_i2c_ioports(void);
>>
>> void __init mpc885ads_board_setup(void)
>> {
>> @@ -120,6 +121,10 @@ #endif
>> #ifdef CONFIG_8XX_SIR
>> init_irda_ioports();
>> #endif
>> +
>> +#ifdef CONFIG_I2C_RPXLITE
>> + init_i2c_ioports();
>> +#endif
>>
>
> Does it hurt to always do it, even when the driver is not enabled? THat'd
> do away with an ifdef.
>
>
Well it will hurt - 8xx has conflicting io pin configurations, and
nothing should be set up "just in case".
> Also, if you move the static function up, you don't need a prototype. That
> goes for other stuff in this file too.
>
>
>> }
>>
>>
>> @@ -361,6 +366,15 @@ static void init_irda_ioports()
>> immr_unmap(cp);
>> }
>>
>> +static void init_i2c_ioports()
>> +{
>> + cpm8xx_t *cp = (cpm8xx_t *)immr_map(im_cpm);
>> +
>> + setbits32(&cp->cp_pbpar, 0x00000030);
>> + setbits32(&cp->cp_pbdir, 0x00000030);
>> + setbits16(&cp->cp_pbodr, 0x0030);
>> +}
>>
>
> Looks like you moved this out of the driver and into the platform
> code. What happens to other platforms where it's used?
>
>
i2c && 8xx combo never work with 2.6 at least in mainstream. That's why
related stuff were scheduled to removal by Jean even,
before I came up with this stuff.
>> +
>> int platform_device_skip(const char *model, int id)
>> {
>> #ifdef CONFIG_MPC8xx_SECOND_ETH_SCC3
>> diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
>> index 419b688..7ecd537 100644
>> --- a/arch/powerpc/sysdev/fsl_soc.c
>> +++ b/arch/powerpc/sysdev/fsl_soc.c
>> @@ -331,7 +331,7 @@ static int __init fsl_i2c_of_init(void)
>> for (np = NULL, i = 0;
>> (np = of_find_compatible_node(np, "i2c", "fsl-i2c")) != NULL;
>> i++) {
>> - struct resource r[2];
>> + struct resource r[3];
>>
>
> Why? No code that uses it has been changed. Is it a bugfix?
>
>
maybe something stray...
>> struct fsl_i2c_platform_data i2c_data;
>> const unsigned char *flags = NULL;
>>
>> @@ -1215,4 +1215,63 @@ err:
>>
>> arch_initcall(fs_irda_of_init);
>>
>> +static const char *i2c_regs = "regs";
>> +static const char *i2c_pram = "pram";
>> +static const char *i2c_irq = "interrupt";
>> +
>> +static int __init fsl_i2c_cpm_of_init(void)
>> +{
>> + struct device_node *np;
>> + unsigned int i;
>> + struct platform_device *i2c_dev;
>> + int ret;
>> +
>> + for (np = NULL, i = 0;
>> + (np = of_find_compatible_node(np, "i2c", "fsl-i2c-cpm")) != NULL;
>> + i++) {
>> + struct resource r[3];
>> + struct fsl_i2c_platform_data i2c_data;
>> +
>> + memset(&r, 0, sizeof(r));
>> + memset(&i2c_data, 0, sizeof(i2c_data));
>> +
>> + ret = of_address_to_resource(np, 0, &r[0]);
>> + if (ret)
>> + goto err;
>> + r[0].name = i2c_regs;
>> +
>> + ret = of_address_to_resource(np, 1, &r[1]);
>> + if (ret)
>> + goto err;
>> + r[1].name = i2c_pram;
>> +
>> + r[2].start = r[2].end = irq_of_parse_and_map(np, 0);
>> + r[2].flags = IORESOURCE_IRQ;
>> + r[2].name = i2c_irq;
>> +
>> + i2c_dev = platform_device_register_simple("fsl-i2c-cpm", i, &r[0], 3);
>> + if (IS_ERR(i2c_dev)) {
>> + ret = PTR_ERR(i2c_dev);
>> + goto err;
>> + }
>> +
>> + ret =
>> + platform_device_add_data(i2c_dev, &i2c_data,
>> + sizeof(struct
>> + fsl_i2c_platform_data));
>> + if (ret)
>> + goto unreg;
>> + }
>> +
>> + return 0;
>> +
>> +unreg:
>> + platform_device_unregister(i2c_dev);
>> +err:
>> + return ret;
>> +}
>> +
>> +arch_initcall(fsl_i2c_cpm_of_init);
>>
>
> This could all be done with an of_platform driver instead, and avoid the above.
> (Someone else already suggested that I believe).
>
>
I know i know. But it was decided, while both ppc/ and powerpc/ wander
around, platform devices way is preferrable.
It is apparent why - so far only mpc885 is alive in arch/powerpc, and it
is not going to change soon for 8xx. OTOH,
some stuff from arch/ppc might use it/add BSP configuration etc.
Having some devices on of_device and some on pdev look kinda messy.
Thanks,
Vitaly
[-- Attachment #2: Type: text/html, Size: 5458 bytes --]
^ permalink raw reply
* Re: [PATCH 3/6] Consolidate of_find_property
From: Matt Sealey @ 2007-04-25 17:46 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Stephen Rothwell, Paul Mackerras, David S. Miller, ppc-dev
In-Reply-To: <c249005137aa3e50c25221f6849a70ea@kernel.crashing.org>
Can't the ata/ATA thing be fixed by simply fixing device trees where it
happens? strncmp seems the standards-compliant route to take.. why clutter
the common parsing routines with fixes for deviant platforms?
The Mac obviously has a few of_platform things where you can add a
compatible search for ata then ATA, keeping it all in the drivers (where
relevant) and in the platform setup (where necessary)
--
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations
Segher Boessenkool wrote:
>>> The question is, what ppc drivers would break if we used strcmp
>>> instead of strcasecmp? I have a dim memory that some Apple machines
>>> had "ata" and others had "ATA" for the hard disk, for instance.
>> ata/ATA and ide/IDE is the main one that comes to mind, but I wouldn't
>> exclude something like Powermac vs. PowerMac or that sort of thing... I
>> remember a case related bug about 2 years ago but can't find what it
>> was ...
>>
>> I think it's safe to settle on strcasecmp for most things for now
>
> Yes, it's hard to imagine any case where strcasecmp(),
> although technically incorrect, would break anything.
> Please document in the code that is _is_ wrong and _why_
> it is done though.
>
>
> Segher
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH] Xilinx framebuffer device driver
From: Andrei Konovalov @ 2007-04-25 18:06 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Rick Moleres, linuxppc-embedded
In-Reply-To: <200704250035.18673.arnd@arndb.de>
Hi Arnd,
Arnd Bergmann wrote:
>> + .activate = FB_ACTIVATE_NOW,
>> + .height = 99, /* in mm of NEC NL6448BC20-08 on ML300 */
>> + .width = 132 /* in mm of NEC NL6448BC20-08 on ML300 */
>> +};
>
> The size looks very specific to a particular display hardware. I guess
> when the driver gets converted to an of_platform_driver, this should come
> from the device tree.
>
> Until then, maybe a config option, with a reasonable default would be
> right.
I could add these to platform data.
Together with the "use DCR access" option.
Say,
--------------------------------------------------------
struct xilinxfb_platform_data {
u32 use_dcr;
u32 screen_height_mm;
u32 screen_width_mm;
};
static struct xilinxfb_platform_data xilinxfb_pdata = {
#if defined(XPAR_TFT_0_USE_DCR) && (XPAR_TFT_0_USE_DCR != 0)
.use_dcr = 1;
#else
.use_dcr = 0;
#endif
.screen_height_mm = CONFIG_FB_XILINX_SCR_HEIGHT;
.screen_width_mm = CONFIG_FB_XILINX_SCR_WIDTH;
};
...
/* ML300/403 reference design framebuffer */
#if defined(XPAR_TFT_0_BASEADDR)
{
.name = "xilinxfb",
.id = 0,
.dev.platform_data = &xilinxfb_pdata,
.num_resources = 1,
.resource = (struct resource[]) {
{
.start = XPAR_TFT_0_BASEADDR,
.end = XPAR_TFT_0_BASEADDR+7,
.flags = IORESOURCE_IO,
},
},
},
#endif
--------------------------------------------------------
Does it look OK?
>> +
>> +struct xilinxfb_drvdata {
>> +
>> + struct fb_info info; /* FB driver info record */
>> +
>> + unsigned long regs_phys; /* phys. address of the control registers */
>> + u32 *regs; /* virt. address of the control registers */
>> +
>> + unsigned char *fb_virt; /* virt. address of the frame buffer */
>
> The virtual addresses should be marked as __iomem, so you can check the correct
> usage with sparse.
OK. Thanks.
> Not sure about regs_phys. Can this be beyond the 32 bit limit on any machine?
The driver is for Xilinx FPGAs only (the video controller is the "soft core" IP
block).
My personal feeling is that someday in the future Virtex FPGAs would carry ppc440 "hard" cores.
But all current FPGAs have only ppc405 "hard core" CPU blocks inside (0, 1 or several per chip).
Another option is to use Microblaze "soft core" CPU which is 32-bit too.
I doubt someone would connect a Virtex FPGA to a "standalone" CPU to use
the FPGA as video controller (this is not impossible though).
I.e. for the moment
u32 regs_phys;
should be OK.
Other options could be
dma_addr_t regs_phys;
or
resource_size_t regs_phys;
which are u32 or u64 depending on the architecture.
But both don't sound like intended specifically for phys. addresses
(not necessary DMA-able).
What would you recommend?
>> +
>> + drvdata = kmalloc(sizeof(struct xilinxfb_drvdata), GFP_KERNEL);
>> + if (!drvdata) {
>> + printk(KERN_ERR "Couldn't allocate device private record\n");
>> + return -ENOMEM;
>> + }
>> + memset((void*)drvdata, 0, sizeof(struct xilinxfb_drvdata));
>
> Use
> drvdata = kzalloc(sizeof (*drvdata), GFP_KERNEL);
Oops.. Sure I will.
>> + drvdata->regs_phys = regs_res->start;
>> + drvdata->regs = (u32 *) ioremap(regs_res->start, 8);
>
> u32 __iomem*
OK
> Arnd <><
Thanks,
Andrei
^ permalink raw reply
* Re: [PATCH] Xilinx framebuffer device driver
From: Andrei Konovalov @ 2007-04-25 18:16 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-embedded
In-Reply-To: <462F98B4.2000108@ru.mvista.com>
Grant,
Andrei Konovalov wrote:
> I could add these to platform data.
> Together with the "use DCR access" option.
> Say,
>
> --------------------------------------------------------
> struct xilinxfb_platform_data {
> u32 use_dcr;
> u32 screen_height_mm;
> u32 screen_width_mm;
> };
>
> static struct xilinxfb_platform_data xilinxfb_pdata = {
> #if defined(XPAR_TFT_0_USE_DCR) && (XPAR_TFT_0_USE_DCR != 0)
> .use_dcr = 1;
> #else
> .use_dcr = 0;
> #endif
IOW I am trying to avoid conditional compilation like:
#if defined(XPAR_TFT_0_USE_DCR) && (XPAR_TFT_0_USE_DCR != 0)
#define xilinxfb_out_be32(addr, mask) mtdcr((addr), (mask))
#else
#define xilinxfb_out_be32(addr, mask) out_be32((addr), (mask))
#endif
- as this would make the driver to include xparameters.h which would
be an issue when moving to the OF device tree.
Or "use DCR" could be a Kconfig option for the driver.
Thanks,
Andrei
^ permalink raw reply
* Re: [PATCH] ppc: Add support for bigger page sizes than 4KB on PPC44x
From: Yuri Tikhonov @ 2007-04-25 17:37 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <17921.3150.242081.66071@cargo.ozlabs.ibm.com>
The patch replaces the CONFIG_PPC_PAGE_SHIFT integer option with choice of
three boolean options
Signed-off-by: Yuri Tikhonov <yur@emcraft.com>
--
diff --git a/arch/ppc/Kconfig b/arch/ppc/Kconfig
index 72af523..5eb58dc 100644
--- a/arch/ppc/Kconfig
+++ b/arch/ppc/Kconfig
@@ -1201,15 +1201,24 @@ config SECCOMP
If unsure, say Y. Only embedded should say N here.
-config PPC_PAGE_SHIFT
- int "Page size (12=>4KB; 14=>16KB; 16=>64KB)" if 44x
- default "12"
- range 12 16
- help
+choice
+ prompt "Page size"
+ default PPC_PAGE_4K
+ ---help---
The PAGE_SIZE definition. Increasing the page size may
improve the system performance in some dedicated cases.
- If unsure, set it to 12 (4KB).
+ If unsure, set it to 4 KB.
+
+config PPC_PAGE_4K
+ bool "4 KB"
+
+config PPC_PAGE_16K
+ bool "16 KB" if 44x
+
+config PPC_PAGE_64K
+ bool "64 KB" if 44x
+endchoice
endmenu
diff --git a/include/asm-ppc/page.h b/include/asm-ppc/page.h
index 0b8f354..f46712e 100644
--- a/include/asm-ppc/page.h
+++ b/include/asm-ppc/page.h
@@ -4,7 +4,13 @@
#include <asm/asm-compat.h>
/* PAGE_SHIFT determines the page size */
-#define PAGE_SHIFT CONFIG_PPC_PAGE_SHIFT
+#if defined(CONFIG_PPC_PAGE_4K)
+#define PAGE_SHIFT 12
+#elif defined(CONFIG_PPC_PAGE_16K)
+#define PAGE_SHIFT 14
+#elif defined(CONFIG_PPC_PAGE_64K)
+#define PAGE_SHIFT 16
+#endif
#define PAGE_SIZE (ASM_CONST(1) << PAGE_SHIFT)
/*
^ permalink raw reply related
* Re: [PATCH] Xilinx framebuffer device driver
From: Grant Likely @ 2007-04-25 18:35 UTC (permalink / raw)
To: Andrei Konovalov; +Cc: linuxppc-embedded
In-Reply-To: <462F9B1A.3090906@ru.mvista.com>
On 4/25/07, Andrei Konovalov <akonovalov@ru.mvista.com> wrote:
> Grant,
>
> > static struct xilinxfb_platform_data xilinxfb_pdata = {
> > #if defined(XPAR_TFT_0_USE_DCR) && (XPAR_TFT_0_USE_DCR != 0)
> > .use_dcr = 1;
> > #else
> > .use_dcr = 0;
> > #endif
>
> IOW I am trying to avoid conditional compilation like:
I agree 100%. All the device options should be specified dynamically.
Heck, it should be possible to build a single kernel and boot it on
just about any design.
>
> #if defined(XPAR_TFT_0_USE_DCR) && (XPAR_TFT_0_USE_DCR != 0)
> #define xilinxfb_out_be32(addr, mask) mtdcr((addr), (mask))
> #else
> #define xilinxfb_out_be32(addr, mask) out_be32((addr), (mask))
> #endif
>
> - as this would make the driver to include xparameters.h which would
> be an issue when moving to the OF device tree.
Which is a bad thing. I believe that the Xilinx folks would like to
use the device tree for the microblaze target also; but there is a
fair bit of work that need to be done before that is feasable.
>
> Or "use DCR" could be a Kconfig option for the driver.
My preference would be for it to be a runtime thing.
--
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox