Messages in this thread | | | Date | Fri, 02 Dec 2005 08:48:12 +0300 | From | Vitaly Wool <> | Subject | Re: [PATCH 2.6-git] SPI core refresh |
| |
David Brownell wrote:
>On Wednesday 30 November 2005 11:17 pm, Vitaly Wool wrote: > > >>Mark Underwood wrote: >> >> >> >>>>However, there also are some advantages of our core compared to David's I'd like to mention >>>> >>>>- it can be compiled as a module >>>> >>>> >>>So can David's. You can use BIOS tables in which case you must compile the SPI core into the >>>kernel but you can also use spi_new_device which allows the SPI core to be built as a module (and >>>is how I am using it). >>> >>> >>You limit the functionality, so it's not the case. >> >> > >As noted in my comparison of last week (you're still ignoring that): > > - Mine lets board-specific device tables be declared in the > relevant arch_setup() thing (board-*.c). Both frameworks allow > later board specific code to dynamically declare the devices, > with binary (Dave's) or parsed-text (Dmitry's) descriptions. > >What Mark said was that in this case he used the "late" init. You seem >to be saying he's not allowed to do that. Which is nonsense; there are >distinct mechanisms for the good reason that "late" init doesn't work >so well without dynamic discovery ... which SPI itself doesn't support. >Hence the need for board-specific "this hardware exists" tables. > > > Can you please clarify what you mean here? Better even if Mark describes what he does. The ideal situation would be if he posted a patch.
> > >>If there's more than one SPI controller onboard, spi_write_then_read >>will serialize the transfers ... >> >> > >Which, as has been pointed out, would be a trivial thing to fix >if anyone were actually to have a problem. Sure it'd incur the >cost of a kmalloc on at least some paths -- serializing in the >slab layer instead! -- but that's one price of using convenience >helpers not performance oriented calls. > > Well, most of the drivers will use that helpers I guess. The thing however is that if you try to implement this in a "clean" way you will come to a sport of framework we've developed for memeory allocations, as I've saild previously.
> > > >> Moreover, if, say, two >>kernel threads with different priorities are working with two SPI >>controllers respectively *priority inversion* will happen. >> >> > >That characteristic being inherited from semaphores (or were they >updated with RT_PREEMPT?), and being in common with most I/O queues >in the system. Not something to blame on any line of code I wrote. > > I think they weren't. The whole thing doesn't seem thought out nicely to me. The solution exists, of course, and that is -- do somthing similar to what we did there.
>Oh, and I noticed a priority inversion in your API which shows >up with one SPI controller managing two devices. Whoops! I'd >far rather have such inversions be implementation artifacts; it's >easy to patch an implementation, hard to change all API users. > > Not sure if I understood you. Can you please describe the situation when this prio inversion happens? What priorities are you talking about? One controller is one thread, so it's _one_ priority, consequently there's nothing to invert. As for your second statement, I don't argue. The fact however is that if you implement the mehtod which corrects priority inverstion problems your core will not be either so lightweight or so flexible. :)
> > > >>>>- it's more adapted for use in real-time environments >>>>- it's not so lightweight, but it leaves less effort for the bus driver developer. >>>> >>>> >>>But also less flexibility. A core layer shouldn't _force_ a policy >>> >>> >>Nope, it's just a default policy. >> >> > >One that every driver pays the price for. Allocating a task even >when it doesn't need it; every call going through a midlayer that >wants to take over queue management policy; and more. (Unless you >made a big un-remarked change in a patch you called "refresh"...) > > It's not obvious that this price is high. Anyway, it's a point I should agree with; this functionality better be a config option. Feel free to submit a patch, as you like to say.
> > > >>>on a bus driver. I am currently developing an adapter driver for David's system and I wouldn't say >>>that the core is making me do things I think the core should do. Please could you provide examples >>>of where you think Davids SPI core requires 'effort'. >>> >>> >>Main are >>- the need to call 'complete' in controller driver >> >> > >So you think it's better to have consistent semantics be optional? > >That seems to be the notion behind your spi_transfer() call, which >can't decide whether it's going to be synchronous or asynchronous. >Instead, it decided to be error prone and be both. :) > > > > Not sure if I understood you here, sorry.
>>- the need to implement policy in controller driver >> >> > >The "policy" in question is something that sometimes needs to >be board-specific -- priority to THAT device, synch with THIS >external signal, etc -- which is why I see it as a drawback >that you insist the core implement one policy. > > Again, the policy can be overridden.
Vitaly - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |