OCP vs. platform_device (was Marvell 64360/64340 GigE driver for MIPS and PPC....)

OCP vs. platform_device (was Marvell 64360/64340 GigE driver for MIPS and PPC....)

[Mark A. Greer]

Moving to a wider PPC audience...

Mark
--

Russell King wrote:

>On Fri, Oct 08, 2004 at 12:16:43PM +0100, David Woodhouse wrote:
>  
>
>>On Fri, 2004-10-08 at 13:13 +0200, Ralf Baechle wrote:
>>    
>>
>>>I was already considering to implement something like OCP for MIPS also.
>>>Since it already exists on PPC I instead would suggest to move
>>>arch/ppc/syslib/ocp.c into generic code, something like drivers/ocp/
>>>maybe?
>>>      
>>>
>>Fine... but if you're doing that instead of extending platform_device to
>>meet your needs, make sure you remove platform_device while you're at it
>>and convert its users to OCP.
>>    
>>
>
>There are a lot of users of platform devices, and this now includes
>virtually everything in the input layer.  This would be a very large
>amount of work to rip out platform devices.
>
>Let's look at this:
>
>struct ocp_def {
>        unsigned int    vendor;
>        unsigned int    function;
>
>These are meaningless for the vast majority of platform devices.
>
>        int             index;
>        phys_addr_t     paddr;
>
>Some platform devices have multiple addresses.  phys_addr_t isn't
>sensible anyway - it really really really wants to be a struct
>resource so that you're integrated into the kernels resource
>management system.
>
>        int             irq;
>
>And have multiple IRQs, and may very well have multiple DMA channel
>numbers.
>
>        unsigned long   pm;
>        void            *additions;
>        void            (*show)(struct device *);
>};
> 
> 
>/* Struct for a given device instance */
>struct ocp_device {
>        struct list_head        link;
>
>Unnecessary - the device model provides this for us already.
>
>        char                    name[80];       /* device name */
>
>Already part of platform_device.
>
>        struct ocp_def          *def;           /* device definition */
>        void                    *drvdata;       /* driver data for this device */
>
>This is a duplication of the driver data in struct device.
>
>        struct ocp_driver       *driver;
>
>Another duplication.
>
>        u32                     current_state;  /* Current operating state. In ACPI-speak,
>                                                   this is D0-D3, D0 being fully functional,
>                                                   and D3 being off. */
>
>and yet again another duplication.
>
>        struct                  device dev;
>};
>
>So I think ripping out platform devices and throwing in something
>which is very different is a backwards step.
>
>  
>

Re: OCP vs. platform_device (was Marvell 64360/64340 GigE driver for MIPS and PPC....)

[Mark A. Greer]

Forwarding to a wider PPC audience...

Mark
--

Dale Farnsworth wrote:

On Fri, Oct 08, 2004 at 08:25:18AM -0400, Brian Waite wrote:

>> Just because I am a bit unclear as to what the concensus let's outline again 
>> the alternative as I see them and see which makes the most sense.
>> 
>> 1) Move ocp to generic code and move MIPS to ocp.
>>  - Not good because there are too many uses of platform_devices and ocp 
>> appears to be an inadequeate solution
>> 
>> 2) Move PPC, or at least Marvell PPC platforms, to the platform_device. 
>>  - I think the above argument can apply.
>> 
>> 3) Keep MIPS on platform_device and PPC on ocp and provide glue logic to stick 
>> either interface to the ethernet driver.
>>  - Not the cleanest solution as it still keep 2 independent interfaces that 
>> essenitally do the same thing in the tree, but it hurst everyone the least 
>> amount to make work.
>> 
>> Of those three options, I am thinking 3 is the way to go. If we can agree on 
>> that then we can start working out the details.
>  
>

I prefer a variation on option 2.  Only support platform_device on the
Marvell ethernet driver and support both OCP and platform_device drivers
on Marvell PPC platforms.

When I added ethernet support for redwood5 and redwood6 in 2.6, I used
the arm smc91x driver, which has a platform_device interface.  I found
it easier to add the platform_device calls to the redwood platform
files than to add an ocp interface to the smc91x driver.  Check out
arch/ppc/platforms/4xx/redwood5.c for a simple platform_device example.

More important than ease of implementation is that I think OCP has
already lost out to platform_device.  (I raised a ruckus on #mklinux
a few months back when I suggested this. :-)  As others have said,
they perform essentially the same function* and platform_device is
already supported in multiple architectures.  OCP must die, but I
prefer a slow death. :-)  We can transition from supporting OCP to
platform_device one driver at a time, by having platforms support
both during the transition.

This doesn't mean I'm thrilled with platform_device.  Shoehorning
everything into a struct resource is ugly.  I also think it needs
to better describe device hierarchies (which OCP doesn't do today
either), which will be a major change.  But platform_device is as
good as OCP now.  The pros and cons between them are nits, trumped
by platform_device's cross-architecture support.

-Dale

* BTW, for the PPC folks, I also think bi_recs and the OF device tree
serve nearly the same function.  IMHO, we should use the same mechanism
to pass device info to the kernel that the kernel uses to pass device
info to the drivers.  But that's extraneous to this discussion, and I'm
not proposing that we do anything on this now.

Re: OCP vs. platform_device (was Marvell 64360/64340 GigE driver for MIPS and PPC....)

[Matt Porter]

On Fri, Oct 08, 2004 at 11:01:11AM -0700, Mark A. Greer wrote:
> Moving to a wider PPC audience...
> 
> Mark
> --
> 
> Russell King wrote:
> 
> >On Fri, Oct 08, 2004 at 12:16:43PM +0100, David Woodhouse wrote:
> >  
> >
> >>On Fri, 2004-10-08 at 13:13 +0200, Ralf Baechle wrote:
> >>    
> >>
> >>>I was already considering to implement something like OCP for MIPS also.
> >>>Since it already exists on PPC I instead would suggest to move
> >>>arch/ppc/syslib/ocp.c into generic code, something like drivers/ocp/
> >>>maybe?
> >>>      
> >>>
> >>Fine... but if you're doing that instead of extending platform_device to
> >>meet your needs, make sure you remove platform_device while you're at it
> >>and convert its users to OCP.
> >>    
> >>
> >
> >There are a lot of users of platform devices, and this now includes
> >virtually everything in the input layer.  This would be a very large
> >amount of work to rip out platform devices.

<snip a bunch of valid points>


> >So I think ripping out platform devices and throwing in something
> >which is very different is a backwards step.

As I've mentioned a couple time now on #mklinux, we should be
moving to platform_device on PPC.  When I moved OCP forward from
2.4 it was because a bunch of work was done to fix it up there
by BenH and the 4xx stuff had been using it for a long time before
platform_device was around. There's a bunch of "backend" type info
that is PPC subarch specific that is encoded in the .additions field
of the OCP defs that can be moved to the platform_data field.  That's
really where most of the important stuff for OCP is at.  We really
just need somebody with time to convert all the subarches to add 
platform_devices and drivers to gather info from them. I may or
may not have time to accomplish this myself in the near future.
 
The point here is that I'm 100% behind converting from ocp_device
to platform_device as long as whoever does it preserves the
rest of the functionality (which should mostly can accomplished
by moving the additions info to platform_data. Maybe it's only
4xx that needs this so other PPC subarches can convert possibly
with a little less work.

-Matt