On Wed, Apr 08, 2009 at 02:58:18PM -0700, David Brownell wrote:
> On Tuesday 07 April 2009, Mark A. Greer wrote:
> > Its better than making a whole bunch of globals and a whole bunch of
> > platform_device_register() calls in every board file. That's
> > unnecessary code bloat. This routine (whatever we call it) makes it
> > simple for board code to register all of the devices it wants
> > registered and without a bazillion globals, etc.
>
> You propose:
>
> - variable length table of { device tag, device-specific data }
> - pass that table to some init function
> - platform_device_register() for e.g. peripherals on EMIF
>
> I said instead:
>
> - variable set of calls to setup_device(device-specific data)
> - platform_device_register() for e.g. peripherals on EMIF
>
> In that sense they're isomorphic, except that you're creating a new
> type framework instead of using the standard C type framework. Your
> new type framework precludes compile-time type checking (and doesn't
> include a runtime replacement), thereby increasing the likelihood of
> init-time errors. (Which are already way too common; better to use
> approaches which *reduce* them.)
>
> I see no code bloat; if anything, there's less because you would need
> extra code to perform the actions implied by that table. The "globals"
> are basically __init code.
Okay, I understand better now. Thanks.
TBH, we've probably spent way more bits on this than its worth.
I think I'll remove da830_register_devices() (and enums), make the
da830 standard platform_device data global, and use
platform_add_devices() in the board code to add them (or just a bunch
of platform_device_register() calls).
> > > I think I gave the example of SPI already. The platform_data should
> > > not cover pinmuxing,
> >
> > I never suggested using platform_data for pinmuxing.
>
> You didn't provide an alternate location for it ... you only
> passed platform_data. Ergo, to the extent you were making a
> complete proposal, you certainly implied that's where it would
> be going. (The issue in this case being which pinmux *options*
> were needed ... like, which chipselects the board uses.)
>
>
> > You're trying to use an implementation problem (that's easily solved) to
> > justify doing pinmux setup at the wrong time/place.
>
> I think that's what *you* are doing. Pinmux should be set
> up before the device node is created. It's a separate problem
> from clk_enable() ... even for cases like CLKOUTx pins.
You misunderstand me. I hope my other email straightens that out.
> Moreover, you keep touching on the notion of detecting pinmux
> conflicts at runtime, but haven't actually solved that problem.
That's easy with a little extra data & support code in mach-davinci.
> Until you solve it for real -- with a database of all pins, and
> other resources, controlled by the DaVinci mux registers, and
> which ones are affected by each mux setting; and a way to keep
> growing that for each new SoC -- you should just remove arguments
> presuming such a solution from your slate.
Okay, it seems like a patch is necessary at this point.
We don't seem to be able to communicate any other way.
> > That makes the most sense, that's what works the best (you can use
> > modules with conflicting resources as long as you don't have both
> > running at the same time), and that's the Linux model.
>
> See above. Lacking any runtime solution to pinmux conflict
> resolution, it's not particularly fair to premise your own
> arguments on the existince of such a solution, is it??
>
>
> > Its unfortunate that the hook name in the drivers ATM is clk_enable().
>
> I really *do not understand* where you're coming from at all.
>
> It's called clk_enable() because -- wait for it!! -- its job
> is to -- here it is!! -- enable the clock!! Surprised??
Nice...
Mark
--
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
