Trivial cleanup in ocp_uart.c

Trivial cleanup in ocp_uart.c

[David Gibson]

ocp_uart.c allocates private space with ocp_alloc_dev, but never uses
it.

diff -urN /home/dgibson/kernel/linuxppc_2_4_devel/arch/ppc/kernel/ocp_uart.c linux-grinch/arch/ppc/kernel/ocp_uart.c
--- /home/dgibson/kernel/linuxppc_2_4_devel/arch/ppc/kernel/ocp_uart.c	Wed Jun 19 10:32:59 2002
+++ linux-grinch/arch/ppc/kernel/ocp_uart.c	Thu Jun 20 17:32:57 2002
@@ -99,7 +99,7 @@
 	int curr_uart = 0;

 	while (curr_uart != -ENXIO) {
-		if (!(uart_drv = ocp_alloc_dev(sizeof (struct serial_struct))))
+		if (!(uart_drv = ocp_alloc_dev(0)))
 			return -ENOMEM;
 		uart_drv->type = UART;
 		/* this returns the next uart number */


--
David Gibson			| For every complex problem there is a
david@gibson.dropbear.id.au	| solution which is simple, neat and
				| wrong.  -- H.L. Mencken
http://www.ozlabs.org/people/dgibson

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: Trivial cleanup in ocp_uart.c

[Tom Rini]

On Thu, Jun 20, 2002 at 05:34:40PM +1000, David Gibson wrote:

> ocp_uart.c allocates private space with ocp_alloc_dev, but never uses
> it.

Er...  So you pass 0 to ocp_alloc_dev() ?  Which will allocate
(sizeof(*ocp_dev) + 0 + 31), so we don't actually break out of this
loop which I take it was your intent.

And I think it can be used, once it gets registered to the ocp_list (and
something later accesses it).

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: Trivial cleanup in ocp_uart.c

[David Gibson]

On Thu, Jun 20, 2002 at 08:50:26AM -0700, Tom Rini wrote:
>
> On Thu, Jun 20, 2002 at 05:34:40PM +1000, David Gibson wrote:
>
> > ocp_uart.c allocates private space with ocp_alloc_dev, but never uses
> > it.
>
> Er...  So you pass 0 to ocp_alloc_dev() ?  Which will allocate
> (sizeof(*ocp_dev) + 0 + 31), so we don't actually break out of this
> loop which I take it was your intent.

No that wasn't my intent.  It's just that we never use the private
space, so there's no point allocating it.  Passing zero is the best we
can do for now (and it's what every other ocp driver except i2c does
now).  Actually I'd like drivers to include struct ocp_dev as part of
their own internal private structures rather than the other way around
(i.e. I think the ocpdev field should go away entirely).
I2C needs some rearranging to do this though (rearranging which will
also let the vaddr field be removed).

> And I think it can be used, once it gets registered to the ocp_list (and
> something later accesses it).

*Nothing* scans the ocp_list (well, except ocp_register() and
ocp_unregister()).  This is why I think ocp_register() is a pointless
interface.

--
David Gibson			| For every complex problem there is a
david@gibson.dropbear.id.au	| solution which is simple, neat and
				| wrong.  -- H.L. Mencken
http://www.ozlabs.org/people/dgibson

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: Trivial cleanup in ocp_uart.c

[Tom Rini]

On Fri, Jun 21, 2002 at 10:52:16AM +1000, David Gibson wrote:
> On Thu, Jun 20, 2002 at 08:50:26AM -0700, Tom Rini wrote:
> > And I think it can be used, once it gets registered to the ocp_list (and
> > something later accesses it).
>
> *Nothing* scans the ocp_list (well, except ocp_register() and
> ocp_unregister()).  This is why I think ocp_register() is a pointless
> interface.

Yes, but you're also judging an incomplete API.  Armin, would things
like PM support at least be scanning the ocp_list ?

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: Trivial cleanup in ocp_uart.c

[David Gibson]

On Fri, Jun 21, 2002 at 07:39:03AM -0700, Tom Rini wrote:
>
> On Fri, Jun 21, 2002 at 10:52:16AM +1000, David Gibson wrote:
> > On Thu, Jun 20, 2002 at 08:50:26AM -0700, Tom Rini wrote:
> > > And I think it can be used, once it gets registered to the ocp_list (and
> > > something later accesses it).
> >
> > *Nothing* scans the ocp_list (well, except ocp_register() and
> > ocp_unregister()).  This is why I think ocp_register() is a pointless
> > interface.
>
> Yes, but you're also judging an incomplete API.  Armin, would things
> like PM support at least be scanning the ocp_list ?

I can't see why it would need to: ocp_register() will just be calling
pm_register() which will directly provide a usable device identifier
to its callbacks.  For that matter I don't see much point in running
the PM stuff through the OCP layer at all rather than just having the
relevant drivers call pm_register() directly.  But the last point
might change if useful default power management handlers could be
implemented through the CPM constants added to core_ocp in Armin's
recent patch.

Incidentally I think a "register" interface could make sense if the
"registration" occured at the point the device was discovered - which
for these embedded devices means unconditionally during board and/or
chip initialization.  This is the obvious way to map embedded devices
onto the 2.5 unified device model.  Registration at driver
initialization like ocp_register() makes no sense to me.

Basically there are two quite distinct approaches either of which
would make sense (for any given peripheral/bus type):
	a) Register each device as it is discovered.  Register each
driver as the appropriate module loads.  Whenever either event happens
the common code calls init or probe callbacks in a driver to connect
it to any devices of appropriate type.
	b) No registration.  When a driver loads, it searches for all
devices of appropriate type, and initializes each of them in turn.

Examples of (a) include the new PCI interface, PCMCIA, and the 2.5
unified device model.  Examples of (b) include the old PCI interface
(pci_find_device()) and (some?/all?) non-PCI devices on OF machines
(e.g. Airport).

Approach (a) is clearly needed to handle hotplug.  But that's not an
issue for OCPs (indeed device (not driver) registration will always be
degenerate) so either approach would do.  Frankly (a) seems overkill
for OCPs, but it does have the advantage of being easily integrated
into the unified device model.

What we have at the moment is mishmash of both models, which *doesn't*
make sense).

--
David Gibson			| For every complex problem there is a
david@gibson.dropbear.id.au	| solution which is simple, neat and
				| wrong.  -- H.L. Mencken
http://www.ozlabs.org/people/dgibson

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: Trivial cleanup in ocp_uart.c

[Scott Anderson]

David Gibson wrote:
> Basically there are two quite distinct approaches either of which
> would make sense (for any given peripheral/bus type):
>         a) Register each device as it is discovered.  Register each
> driver as the appropriate module loads.  Whenever either event happens
> the common code calls init or probe callbacks in a driver to connect
> it to any devices of appropriate type.
>         b) No registration.  When a driver loads, it searches for all
> devices of appropriate type, and initializes each of them in turn.
>
> Examples of (a) include the new PCI interface, PCMCIA, and the 2.5
> unified device model.  Examples of (b) include the old PCI interface
> (pci_find_device()) and (some?/all?) non-PCI devices on OF machines
> (e.g. Airport).
>
> Approach (a) is clearly needed to handle hotplug.  But that's not an
> issue for OCPs (indeed device (not driver) registration will always be
> degenerate) so either approach would do.  Frankly (a) seems overkill
> for OCPs, but it does have the advantage of being easily integrated
> into the unified device model.

I wouldn't be quite so quick to dismiss the issue of hotplug from OCP.
I have heard rumblings of people having FPGAs with on-chip processors
wanting to reprogram the FPGAs on the fly with different sets of
peripherals.

    Scott Anderson
    scott_anderson@mvista.com   MontaVista Software Inc.
    (408)328-9214               1237 East Arques Ave.
    http://www.mvista.com       Sunnyvale, CA  94085

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: Trivial cleanup in ocp_uart.c

[David Gibson]

On Wed, Jun 26, 2002 at 10:27:01AM -0700, Scott Anderson wrote:
>
> David Gibson wrote:
> > Basically there are two quite distinct approaches either of which
> > would make sense (for any given peripheral/bus type):
> >         a) Register each device as it is discovered.  Register each
> > driver as the appropriate module loads.  Whenever either event happens
> > the common code calls init or probe callbacks in a driver to connect
> > it to any devices of appropriate type.
> >         b) No registration.  When a driver loads, it searches for all
> > devices of appropriate type, and initializes each of them in turn.
> >
> > Examples of (a) include the new PCI interface, PCMCIA, and the 2.5
> > unified device model.  Examples of (b) include the old PCI interface
> > (pci_find_device()) and (some?/all?) non-PCI devices on OF machines
> > (e.g. Airport).
> >
> > Approach (a) is clearly needed to handle hotplug.  But that's not an
> > issue for OCPs (indeed device (not driver) registration will always be
> > degenerate) so either approach would do.  Frankly (a) seems overkill
> > for OCPs, but it does have the advantage of being easily integrated
> > into the unified device model.
>
> I wouldn't be quite so quick to dismiss the issue of hotplug from OCP.
> I have heard rumblings of people having FPGAs with on-chip processors
> wanting to reprogram the FPGAs on the fly with different sets of
> peripherals.

Eek, wibble.  It still seems somewhat unlikely to me that you'd be
changing the peripherals "on the fly" in a real life embedded
application.  Especially considering that reboots are likely to be
much less of an issue on an embedded system than on a big server.

--
David Gibson			| For every complex problem there is a
david@gibson.dropbear.id.au	| solution which is simple, neat and
				| wrong.  -- H.L. Mencken
http://www.ozlabs.org/people/dgibson

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: Trivial cleanup in ocp_uart.c

[Scott Anderson]

David Gibson wrote:
> Eek, wibble.  It still seems somewhat unlikely to me that you'd be
> changing the peripherals "on the fly" in a real life embedded
> application.  Especially considering that reboots are likely to be
> much less of an issue on an embedded system than on a big server.

My contrived example is a piece of test equipment that has an LCD
touchscreen and some high speed serial collection device.  The LCD is
only used when interacting with the UI and the serial is only used
during a run when the LCD can be blank.  By reprogramming part of the
FPGA on the fly you can squeeze more into a smaller FPGA.  It appears
that Xilinx already has this working:
  http://www.xilinx.com/xapp/xapp290.pdf

I guess I wouldn't be quite so quick to dismiss this as unlikely, but
I must admit, my crystal ball is in the shop this week.  It sure would
be nice to keep such things in mind as OCP is evolving, though.

    Scott Anderson
    scott_anderson@mvista.com   MontaVista Software Inc.
    (408)328-9214               1237 East Arques Ave.
    http://www.mvista.com       Sunnyvale, CA  94085

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: Trivial cleanup in ocp_uart.c

[Kenneth Johansson]

On Thu, 2002-06-27 at 18:23, Scott Anderson wrote:
>
> David Gibson wrote:
> > Eek, wibble.  It still seems somewhat unlikely to me that you'd be
> > changing the peripherals "on the fly" in a real life embedded
> > application.  Especially considering that reboots are likely to be
> > much less of an issue on an embedded system than on a big server.

> I guess I wouldn't be quite so quick to dismiss this as unlikely, but
> I must admit, my crystal ball is in the shop this week.  It sure would
> be nice to keep such things in mind as OCP is evolving, though.

As someone who actually work with FPGA chips I can assure everyone that
those are always to small even when you pick one that seems insanely
large when you start. I have many times been in a situation when is
could work quite well to have mutually exclusive subsystem but what has
stopped that before is that the chips/software usually do not support
partial reconfiguration and you usually can't reconfigure the whole part
without disturbing normal functions.

--
Kenneth Johansson
Ericsson AB                       Tel: +46 8 404 71 83
Borgafjordsgatan 9                Fax: +46 8 404 72 72
164 80 Stockholm                  kenneth.johansson@etx.ericsson.se


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: Trivial cleanup in ocp_uart.c

[David Gibson]

On Thu, Jun 27, 2002 at 09:23:41AM -0700, Scott Anderson wrote:
>
> David Gibson wrote:
> > Eek, wibble.  It still seems somewhat unlikely to me that you'd be
> > changing the peripherals "on the fly" in a real life embedded
> > application.  Especially considering that reboots are likely to be
> > much less of an issue on an embedded system than on a big server.
>
> My contrived example is a piece of test equipment that has an LCD
> touchscreen and some high speed serial collection device.  The LCD is
> only used when interacting with the UI and the serial is only used
> during a run when the LCD can be blank.  By reprogramming part of the
> FPGA on the fly you can squeeze more into a smaller FPGA.  It appears
> that Xilinx already has this working:
>   http://www.xilinx.com/xapp/xapp290.pdf
>
> I guess I wouldn't be quite so quick to dismiss this as unlikely, but
> I must admit, my crystal ball is in the shop this week.  It sure would
> be nice to keep such things in mind as OCP is evolving, though.

Heh, well, maybe.  In any case this sounds more like a 2.5 and later
timerframe thing to me, in which case integration into the unified
device model should give us what's needed.

--
David Gibson			| For every complex problem there is a
david@gibson.dropbear.id.au	| solution which is simple, neat and
				| wrong.  -- H.L. Mencken
http://www.ozlabs.org/people/dgibson

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: Trivial cleanup in ocp_uart.c

[Tom Rini]

On Fri, Jun 28, 2002 at 10:59:08AM +1000, David Gibson wrote:
> On Thu, Jun 27, 2002 at 09:23:41AM -0700, Scott Anderson wrote:
> >
> > David Gibson wrote:
> > > Eek, wibble.  It still seems somewhat unlikely to me that you'd be
> > > changing the peripherals "on the fly" in a real life embedded
> > > application.  Especially considering that reboots are likely to be
> > > much less of an issue on an embedded system than on a big server.
> >
> > My contrived example is a piece of test equipment that has an LCD
> > touchscreen and some high speed serial collection device.  The LCD is
> > only used when interacting with the UI and the serial is only used
> > during a run when the LCD can be blank.  By reprogramming part of the
> > FPGA on the fly you can squeeze more into a smaller FPGA.  It appears
> > that Xilinx already has this working:
> >   http://www.xilinx.com/xapp/xapp290.pdf
> >
> > I guess I wouldn't be quite so quick to dismiss this as unlikely, but
> > I must admit, my crystal ball is in the shop this week.  It sure would
> > be nice to keep such things in mind as OCP is evolving, though.
>
> Heh, well, maybe.  In any case this sounds more like a 2.5 and later
> timerframe thing to me, in which case integration into the unified
> device model should give us what's needed.

I think alot of what we're talking about for OCP is 2.5 fodder really.
Keep in mind that the current stuff has been well tested on ~13
different 4xx boards.  Perhaps we should allow OCP to become semi solid
in 2_4_devel now so that we can move 4xx out to Marcelo and allow armin
to do some drastic changes in 2.5...

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: Trivial cleanup in ocp_uart.c

[Armin Kuster]

David Gibson wrote:
> On Fri, Jun 21, 2002 at 07:39:03AM -0700, Tom Rini wrote:
>
>>On Fri, Jun 21, 2002 at 10:52:16AM +1000, David Gibson wrote:
>>
>>>On Thu, Jun 20, 2002 at 08:50:26AM -0700, Tom Rini wrote:
>>>
>>>>And I think it can be used, once it gets registered to the ocp_list (and
>>>>something later accesses it).
>>>>
>>>
> I can't see why it would need to: ocp_register() will just be calling
> pm_register() which will directly provide a usable device identifier
> to its callbacks.  For that matter I don't see much point in running
> the PM stuff through the OCP layer at all rather than just having the
> relevant drivers call pm_register() directly.  But the last point
> might change if useful default power management handlers could be
> implemented through the CPM constants added to core_ocp in Armin's
> recent patch.
>
> Incidentally I think a "register" interface could make sense if the
> "registration" occured at the point the device was discovered - which
> for these embedded devices means unconditionally during board and/or
> chip initialization.  This is the obvious way to map embedded devices
> onto the 2.5 unified device model.  Registration at driver
> initialization like ocp_register() makes no sense to me.

Thats obvious			^^^^^^^^^^^^^^^^^^

>
> Basically there are two quite distinct approaches either of which
> would make sense (for any given peripheral/bus type):
> 	a) Register each device as it is discovered.  Register each
> driver as the appropriate module loads.  Whenever either event happens
> the common code calls init or probe callbacks in a driver to connect
> it to any devices of appropriate type.
> 	b) No registration.  When a driver loads, it searches for all
> devices of appropriate type, and initializes each of them in turn.

I'll need to fix that won't I :)

>
> Examples of (a) include the new PCI interface, PCMCIA, and the 2.5
> unified device model.  Examples of (b) include the old PCI interface
> (pci_find_device()) and (some?/all?) non-PCI devices on OF machines
> (e.g. Airport).
>
> Approach (a) is clearly needed to handle hotplug.  But that's not an
> issue for OCPs (indeed device (not driver) registration will always be
> degenerate) so either approach would do.  Frankly (a) seems overkill
> for OCPs, but it does have the advantage of being easily integrated
> into the unified device model.

It's not over kill.  if I used the entire PCI model, it would be.

>
> What we have at the moment is mishmash of both models, which *doesn't*
> make sense).
>
This is in devel, I guess what I should have done was fork and do all
the work there and then provide a complete solution to devel. :)

>


armin


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: Trivial cleanup in ocp_uart.c

[Paul Mackerras]

Armin Kuster writes:

> This is in devel, I guess what I should have done was fork and do all
> the work there and then provide a complete solution to devel. :)

No, working in devel is fine.  But it would help if you could write
down your thoughts about where this is going, and where you would
like it to end up, and post them to the list.

Paul.

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: Trivial cleanup in ocp_uart.c

[Kenneth Johansson]

On Thu, 2002-06-27 at 22:30, Paul Mackerras wrote:
>
> Armin Kuster writes:
>
> > This is in devel, I guess what I should have done was fork and do all
> > the work there and then provide a complete solution to devel. :)
>
> No, working in devel is fine.  But it would help if you could write
> down your thoughts about where this is going, and where you would
> like it to end up, and post them to the list.

And I would very much like to get a clarification how stuff gets moved
from devel to 2.4. What is the criteria used here?? I got the impression
when it was started that it was intended as a testing area before moving
it into 2.4 but the moving it seems to be very slow.

Should perhaps a change that has been in devel for month without ending
up in 2.4 be moved to 2.5.


--
Kenneth Johansson
Ericsson AB                       Tel: +46 8 404 71 83
Borgafjordsgatan 9                Fax: +46 8 404 72 72
164 80 Stockholm                  kenneth.johansson@etx.ericsson.se


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/