we have guys who maintain xml descriptions of all Blackfin processors.
these include an exhaustive list of all the peripherals and their
MMRs. for example, there is an element that binds the address, the
MMR name (as described in the processor's hardware manual), the bit
size of it, and all those fun details. i feed these xml files through
an xsl to generate a C file that populates /sys/kernel/debug/blackfin/
by using the standard debugfs functions. so when i'm debugging the
watchdog driver, i can simply go into
/sys/kernel/debug/blackfin/watchdog/ and see the watchdog-specific
MMRs and cat/echo them on the fly. it's been quite handy and feedback
from customers/developers is that it's nicely filled a small void in
the development process.
the trouble is that this file currently weighs in at ~1.8 megs. this
is because it contains all the information for all Blackfin processors
we support (which currently, is about ~23 variants). it's only going
to get bigger as we support more. Bryan cringes at the thought of
submitting it to LKML :). so i'm fishing around for alternatives ...
the code was originally developed against 2.6.21, so UIO was not a
possibility. i'm still not sure if it is ... i'd have to research it
a bit more and play with things.
any pointers ?
-mike
On Jan 28, 2008 5:16 AM, Mike Frysinger <[email protected]> wrote:
> the trouble is that this file currently weighs in at ~1.8 megs. this
> is because it contains all the information for all Blackfin processors
> we support (which currently, is about ~23 variants). it's only going
> to get bigger as we support more. Bryan cringes at the thought of
> submitting it to LKML :).
erp, forgot to include this ... for the curious, here is the current file:
http://blackfin.uclinux.org/gf/project/linux-kernel/scmsvn/?action=browse&path=/*checkout*/trunk/arch/blackfin/kernel/debug-mmrs.c
-mike
On Mon, 2008-01-28 at 05:16 -0500, Mike Frysinger wrote:
> we have guys who maintain xml descriptions of all Blackfin processors.
> these include an exhaustive list of all the peripherals and their
> MMRs. for example, there is an element that binds the address, the
> MMR name (as described in the processor's hardware manual), the bit
> size of it, and all those fun details. i feed these xml files through
> an xsl to generate a C file that populates /sys/kernel/debug/blackfin/
> by using the standard debugfs functions. so when i'm debugging the
> watchdog driver, i can simply go into
> /sys/kernel/debug/blackfin/watchdog/ and see the watchdog-specific
> MMRs and cat/echo them on the fly. it's been quite handy and feedback
> from customers/developers is that it's nicely filled a small void in
> the development process.
>
Mike is a quick developer, I am trying to keep up with him.
Exactly, the MMR debug FS interface helps very much for developers and
customers.
> the trouble is that this file currently weighs in at ~1.8 megs. this
> is because it contains all the information for all Blackfin processors
> we support (which currently, is about ~23 variants). it's only going
> to get bigger as we support more. Bryan cringes at the thought of
> submitting it to LKML :). so i'm fishing around for alternatives ...
> the code was originally developed against 2.6.21, so UIO was not a
> possibility. i'm still not sure if it is ... i'd have to research it
> a bit more and play with things.
>
The main reason I am not willing to submit this to mainline is the file
size. It's almost the biggest file in the kernel source. And it will be
bigger and bigger when more and more new Blackfin processors supported
by Linux kernel.
My suggestion is:
- move the large MMR table to user space
- add an interface of debug FS which let uses space app to send the large table to debug FS
- debug FS will take care of the rest HW things according to each architecture.
Or more deeper thought:
- we don't need all the MMR setup at the same time for debugging. for example, maybe for some developer, he/she only needs one driver MMR for debugging such as watchdog/usb/spi/i2c ....
- How about split the debug MMR table to each drivers or processors?
- watchdog driver implements a debug FS interface for debugging watchdog MMR and other drivers implement their own things.
I am not familiar with UIO, is it easy interactive with debug FS?
Thanks
-Bryan Wu
On Jan 28, 2008 5:40 AM, Bryan Wu <[email protected]> wrote:
> On Mon, 2008-01-28 at 05:16 -0500, Mike Frysinger wrote:
> > the trouble is that this file currently weighs in at ~1.8 megs. this
> > is because it contains all the information for all Blackfin processors
> > we support (which currently, is about ~23 variants). it's only going
> > to get bigger as we support more. Bryan cringes at the thought of
> > submitting it to LKML :). so i'm fishing around for alternatives ...
> > the code was originally developed against 2.6.21, so UIO was not a
> > possibility. i'm still not sure if it is ... i'd have to research it
> > a bit more and play with things.
>
> The main reason I am not willing to submit this to mainline is the file
> size. It's almost the biggest file in the kernel source. And it will be
> bigger and bigger when more and more new Blackfin processors supported
> by Linux kernel.
a quick check of current git shows it is significantly larger than any other ;)
> My suggestion is:
> Or more deeper thought:
> - we don't need all the MMR setup at the same time for debugging. for example, maybe for some developer, he/she only needs one driver MMR for debugging such as watchdog/usb/spi/i2c ....
splitting things up doesnt really address the original issue: there's
a lot of info here to be kept in the kernel
> - How about split the debug MMR table to each drivers or processors?
> - watchdog driver implements a debug FS interface for debugging watchdog MMR and other drivers implement their own things.
this had been mentioned before as a possibility but shot down. you do
not want to tie the creation of these debug files to anything as the
prevents independent development of any other drivers/application that
use the same peripheral.
-mike
Mike Frysinger wrote:
> On Jan 28, 2008 5:40 AM, Bryan Wu <[email protected]> wrote:
>> On Mon, 2008-01-28 at 05:16 -0500, Mike Frysinger wrote:
>>> the trouble is that this file currently weighs in at ~1.8 megs. this
>>> is because it contains all the information for all Blackfin processors
>>> we support (which currently, is about ~23 variants). it's only going
>>> to get bigger as we support more. Bryan cringes at the thought of
>>> submitting it to LKML :). so i'm fishing around for alternatives ...
>>> the code was originally developed against 2.6.21, so UIO was not a
>>> possibility. i'm still not sure if it is ... i'd have to research it
>>> a bit more and play with things.
>> The main reason I am not willing to submit this to mainline is the file
>> size. It's almost the biggest file in the kernel source. And it will be
>> bigger and bigger when more and more new Blackfin processors supported
>> by Linux kernel.
>
> a quick check of current git shows it is significantly larger than any other ;)
>
>> My suggestion is:
>> Or more deeper thought:
>> - we don't need all the MMR setup at the same time for debugging. for example, maybe for some developer, he/she only needs one driver MMR for debugging such as watchdog/usb/spi/i2c ....
>
> splitting things up doesnt really address the original issue: there's
> a lot of info here to be kept in the kernel
>
>> - How about split the debug MMR table to each drivers or processors?
>> - watchdog driver implements a debug FS interface for debugging watchdog MMR and other drivers implement their own things.
>
> this had been mentioned before as a possibility but shot down. you do
> not want to tie the creation of these debug files to anything as the
> prevents independent development of any other drivers/application that
> use the same peripheral.
> -mike
Hi Mike,
there is a lot of duplication in your file, but you could slim it down a
bit if thats the only objection.
for instance all the COUNTER element addresses have the same offsets
from CNT_CONFIG
CNT_IMASK = CNT_CONFIG + 4
CNT_STATUS = CNT_CONFIG + 8
CNT_COMMAND = CNT_CONFIG + 12
CNT_DEBOUNCE = CNT_CONFIG + 16
CNT_COUNTER = CNT_CONFIG + 20
CNT_MAX = CNT_CONFIG + 24
CNT_MIN = CNT_CONFIG + 28
so you could have a simple function to create all the COUNTER elements
from a given base address, then each variant only needs call that saving
you lots of LOC.
something like :-
make_counter_dir(top, base ) {
parent = debugfs_create_dir("Counter", top);
debugfs_create_x16("CNT_COMMAND", 0600, parent, base+12);
debugfs_create_x16("CNT_CONFIG", 0600, parent, base);
...
}
Hopefully all the other sections have similar levels of duplication, but
I haven't checked.
Cheers
Richard
On Jan 28, 2008 8:04 AM, richard kennedy <[email protected]> wrote:
> Mike Frysinger wrote:
> > On Jan 28, 2008 5:40 AM, Bryan Wu <[email protected]> wrote:
> >> On Mon, 2008-01-28 at 05:16 -0500, Mike Frysinger wrote:
> >>> the trouble is that this file currently weighs in at ~1.8 megs. this
> >>> is because it contains all the information for all Blackfin processors
> >>> we support (which currently, is about ~23 variants). it's only going
> >>> to get bigger as we support more. Bryan cringes at the thought of
> >>> submitting it to LKML :). so i'm fishing around for alternatives ...
> >>> the code was originally developed against 2.6.21, so UIO was not a
> >>> possibility. i'm still not sure if it is ... i'd have to research it
> >>> a bit more and play with things.
> >> The main reason I am not willing to submit this to mainline is the file
> >> size. It's almost the biggest file in the kernel source. And it will be
> >> bigger and bigger when more and more new Blackfin processors supported
> >> by Linux kernel.
> >
> > a quick check of current git shows it is significantly larger than any other ;)
> >
> >> My suggestion is:
> >> Or more deeper thought:
> >> - we don't need all the MMR setup at the same time for debugging. for example, maybe for some developer, he/she only needs one driver MMR for debugging such as watchdog/usb/spi/i2c ....
> >
> > splitting things up doesnt really address the original issue: there's
> > a lot of info here to be kept in the kernel
> >
> >> - How about split the debug MMR table to each drivers or processors?
> >> - watchdog driver implements a debug FS interface for debugging watchdog MMR and other drivers implement their own things.
> >
> > this had been mentioned before as a possibility but shot down. you do
> > not want to tie the creation of these debug files to anything as the
> > prevents independent development of any other drivers/application that
> > use the same peripheral.
>
> there is a lot of duplication in your file, but you could slim it down a
> bit if thats the only objection.
i imagine there's a ton of duplication ... the file is auto-generated
from XML files, so i could take a look at the autogeneration producing
unified code.
> so you could have a simple function to create all the COUNTER elements
> from a given base address, then each variant only needs call that saving
> you lots of LOC.
also a possibility ... just have to be wary of the parts that have
slightly different peripherals (like the UART).
-mike
On Mon, 28 Jan 2008, Mike Frysinger wrote:
> On Jan 28, 2008 8:04 AM, richard kennedy <[email protected]> wrote:
> > Mike Frysinger wrote:
> > > On Jan 28, 2008 5:40 AM, Bryan Wu <[email protected]> wrote:
> > >> On Mon, 2008-01-28 at 05:16 -0500, Mike Frysinger wrote:
> > >>> the trouble is that this file currently weighs in at ~1.8 megs. this
> > >>> is because it contains all the information for all Blackfin processors
> > >>> we support (which currently, is about ~23 variants). it's only going
> > >>> to get bigger as we support more. Bryan cringes at the thought of
> > >>> submitting it to LKML :). so i'm fishing around for alternatives ...
> > >>> the code was originally developed against 2.6.21, so UIO was not a
> > >>> possibility. i'm still not sure if it is ... i'd have to research it
> > >>> a bit more and play with things.
> > >> The main reason I am not willing to submit this to mainline is the file
> > >> size. It's almost the biggest file in the kernel source. And it will be
> > >> bigger and bigger when more and more new Blackfin processors supported
> > >> by Linux kernel.
> > >
> > > a quick check of current git shows it is significantly larger than any other ;)
> > >
> > >> My suggestion is:
> > >> Or more deeper thought:
> > >> - we don't need all the MMR setup at the same time for debugging. for example, maybe for some developer, he/she only needs one driver MMR for debugging such as watchdog/usb/spi/i2c ....
> > >
> > > splitting things up doesnt really address the original issue: there's
> > > a lot of info here to be kept in the kernel
> > >
> > >> - How about split the debug MMR table to each drivers or processors?
> > >> - watchdog driver implements a debug FS interface for debugging watchdog MMR and other drivers implement their own things.
> > >
> > > this had been mentioned before as a possibility but shot down. you do
> > > not want to tie the creation of these debug files to anything as the
> > > prevents independent development of any other drivers/application that
> > > use the same peripheral.
> >
> > there is a lot of duplication in your file, but you could slim it down a
> > bit if thats the only objection.
>
> i imagine there's a ton of duplication ... the file is auto-generated
> from XML files, so i could take a look at the autogeneration producing
> unified code.
Could you submit the XML files and the autogeneration code? The C file
isn't really source. Not only is it big, it'll probably change around a
whole lot when you make small changes to your process, be hard to review,
etc.
-Daniel
*This .sig left intentionally blank*
On Jan 28, 2008 7:08 PM, Daniel Barkalow <[email protected]> wrote:
> On Mon, 28 Jan 2008, Mike Frysinger wrote:
> > On Jan 28, 2008 8:04 AM, richard kennedy <[email protected]> wrote:
> > > there is a lot of duplication in your file, but you could slim it down a
> > > bit if thats the only objection.
> >
> > i imagine there's a ton of duplication ... the file is auto-generated
> > from XML files, so i could take a look at the autogeneration producing
> > unified code.
>
> Could you submit the XML files and the autogeneration code? The C file
> isn't really source. Not only is it big, it'll probably change around a
> whole lot when you make small changes to your process, be hard to review,
> etc.
that would require the build system to have xml tools installed ...
that doesnt sound pleasant.
that said, the XML files in question are probably 10x+ the size of the
C file. swapping 1 meg for 10+ megs ? :)
-mike
On Mon, 28 Jan 2008, Mike Frysinger wrote:
> On Jan 28, 2008 7:08 PM, Daniel Barkalow <[email protected]> wrote:
> > Could you submit the XML files and the autogeneration code? The C file
> > isn't really source. Not only is it big, it'll probably change around a
> > whole lot when you make small changes to your process, be hard to review,
> > etc.
>
> that would require the build system to have xml tools installed ...
> that doesnt sound pleasant.
If they're only required for building blackfin debugging stuff, that
shouldn't be a big deal. People building embedded kernels with debugging
from source can probably handle the extra requirement. Setting up a
cross-compilation toolchain for embedded processors is much trickier than
getting xml tools.
> that said, the XML files in question are probably 10x+ the size of the
> C file. swapping 1 meg for 10+ megs ? :)
If it's a bunch of smaller files, and if changes tend to be localized,
that would be a good tradeoff. Alternatively, have them packaged
separately, which might be more appropriate anyway if people might want to
use them for other purposes (on the host when using jtag, perhaps).
-Daniel
*This .sig left intentionally blank*