Linux IIO development
 help / color / mirror / Atom feed
* Re: [PATCH] Add STMicroelectronics LPS001WP pressure sensor device driver into misc
       [not found]     ` <201103151038.40559.arnd@arndb.de>
@ 2011-03-15 11:11       ` Jonathan Cameron
  2011-03-15 12:51         ` Mark Brown
  2011-03-15 13:29         ` Arnd Bergmann
  0 siblings, 2 replies; 10+ messages in thread
From: Jonathan Cameron @ 2011-03-15 11:11 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jean Delvare, mems applications, rdunlap, carmine.iascone,
	matteo.dameno, rubini, linux-doc, linux-kernel, Guenter Roeck,
	Greg KH, linux-iio@vger.kernel.org

cc'd linux-iio as the discussion is moving more into that realm by the minute!

On 03/15/11 09:38, Arnd Bergmann wrote:
> On Tuesday 15 March 2011 00:23:59 Jonathan Cameron wrote:
>>>
>>> This is indeed the major disadvantage. IIO seems to take a lot of time
>>> to move out of staging, although I don't know what the current ETA is.
>>>
>> Whilst I'd like to say soon there are a couple more big changes to make
>> (such as the ongoing changes to allow threaded interrupts for the triggers
>> - basically implementing what Thomas Gleixner suggested and having virtual
>> interrupt chips) and some of these will take a fair bit of bedding down. In the
>> original write I got quite a few things wrong :( Mind you quite a lot of
>> evolution of requirements has taken place as well.
>>
>> Usual sob story (50+ drivers of mixed quality to avoid breaking,
>> little manpower, steady stream of new drivers keeping people busy reviewing
>> etc etc)
>>
>> Having said that, for the simple drivers the interfaces are mostly fixed
>> and nothing much has changed for quite a few months. (e.g. anything that
>> is slow and only uses hwmon style sysfs interfaces).
>>
>> Of course, if anyone does have any time (looks around hopefully...)
>> Particularly helpful are people pointing out what really needs fixing
>> before attempting to move out of staging.   Much better if we get that
>> sort of feedback in plenty of time rather than on sending a pull request
>> to Linus. I like to spread my arguments out ;)
> 
> Do you think it would help to split the iio codebase into a smaller part
> for the relatively clean drivers that can be put into shape for
> drivers/iio, and the bulk of the code that stays in staging for a bit
> longer, until it gets converted to the new one in small chunks?
> 
> That may be less frustrating than trying to do it all at once. In particular,
> you could do major internal rewrites much faster if you don't have
> to immediately worry about breaking dozens of drivers in the process.
> 
Hi Arnd,

An interesting idea though I'm not entirely sure how it would work in practice.
Two options occurred whilst cycling in this morning.
1) Spit functionality out in staging. This would give a core set that is basically the sysfs
only stuff.  To do that we'd have to define a struct iio_dev_basic and make
it an element of the iio_dev.  Prior to that we'd probably
need to make pretty much all accesses into iio_dev via macros / inline functions which
would not be a trivial undertaking.

Then we could switch those drivers doing the minimum to the _basic form.  At that point
we could perhaps attempt to move a couple of drivers and the abi docs out of staging.

The disadvantages of this that come to mind are:
* Makes the path to driver addition that I'd prefer trickier.  You write a basic sysfs
only driver first, then add on stuff like events and buffering as separate patches. We could
go the other way around like v4l2-subdev and have a base structure with the option
of pointers to structures offering different combinations of features.
* Not many of the drivers I'd consider to be ready to go at the moment are actually in
this _basic class. 

2) Basically make a copy.  This would look like the original patch set did when we went
into staging.  I'd imagine any attempt to move the core wholesale is going to get
kicked back as it would be unreviewably large.  We wouldn't be far from this anyway,
it is just a question of ordering and timing.  To achieve a partial merge we would:

* Take out a subset of struct iio_dev (and associated functions etc), initially just doing
the 'bus' handling and sysfs direct access to the devices.
* Move over the relevant parts of Docs (mostly from sysfs-iio-bus)
* Hack out the relevant chunks of a number of the cleaner drivers.
(the driver and doc moves would occur in sets covering different types of devices to keep
the review burden of looking at the interface to a minimum. We could also clean up the
remaining poorly defined abis as we do this (energy meters and resolvers IIRC).

Pause for a while (note that we would have to rigorously prevent any divergence between the
two versions).

* Copy over the events infrastructure
* Move the next chunk of Docs
* Lift over the events support from clean drivers.

Pause for another while

* Lift the core buffering support
* Lift over the reset of the documentation.
* Take over the hardware buffered device drivers
* Take over kfifo wrapping code (has an inferior feature set to ring_sw but easier to review
by far).
* synchronise the remains of our good driver set and kill them off in staging.

That would leave the ugly drivers in staging to pull over as they get fixed up.

Disadvantages of this:

* It's not necessarily trivial to break up complex drivers into the three elements described here.
They have a tedious habit of interacting.  The issue would be that a lot of the code would seem
like it has been done in a very strange way to any reviewer of the parts they see at a time.
Normally they get to look on to a later patch to see why stuff has been done as it has.  Here, would
anyone really take a look at the driver in staging to understand what is going on?

* The difficulty of keeping the stuff in staging in sync with the stuff outside.

* Fiddly stuff to handle the fact we will for a while have two drivers for an awful lot of stuff.

The alternative would be to do much as above over a much shorter timescale once we are happy with
what we have in staging.  Only substantial other difference is that we take very few drivers along
the intermediate steps (probably just the ones I or whoever puts the patch set together can actually
verify at each step).  We then bring the others over at the end.  This approach would probably happen
into a tree picked up by linux-next over weeks or months.

Perhaps some pseudo version of above would work review wise.  Post the patches in sets and get
acks for each step from all interested players.  After ALS I'd definitely like to get a view
from Linus as early as possible, but I don't think we are ready to approach him quite yet.  There
are big ugly corners that will distract from the main issues.

Thanks,

Jonathan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add STMicroelectronics LPS001WP pressure sensor device driver into misc
       [not found]     ` <201103151024.26436.arnd@arndb.de>
@ 2011-03-15 11:30       ` Jonathan Cameron
  2011-03-15 13:58         ` Arnd Bergmann
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2011-03-15 11:30 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Guenter Roeck, Jean Delvare, mems applications,
	rdunlap@xenotime.net, carmine.iascone@st.com,
	matteo.dameno@st.com, rubini@ipvvis.unipv.it,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-iio@vger.kernel.org

On 03/15/11 09:24, Arnd Bergmann wrote:
> On Monday 14 March 2011 23:48:59 Guenter Roeck wrote:
>> On Mon, Mar 14, 2011 at 05:42:44PM -0400, Jean Delvare wrote:
>>> On Mon, 14 Mar 2011 21:36:43 +0100, Arnd Bergmann wrote:
>>>> On Monday 14 March 2011 21:18:09 Jean Delvare wrote:
>>>>> Jonathan is correct. Pressure sensors are not hardware monitoring
>>>>> devices, their drivers have nothing to do in drivers/hwmon. This is
>>>>> something for drivers/misc or staging/iio.
>>>>
>>>> I generally try to prevent people from adding more ad-hoc interfaces
>>>> to drivers/misc. Anything that is called a drivers/misc driver to me
>>>> must qualify as "there can't possibly be a second driver with the
>>>> same semantics", otherwise it should be part of another subsystem
>>>> with clear rules, or be put into its own file system.
>>>
>>> I see drivers/misc differently. I see it as "not enough drivers of the
>>> same type to justify a new subsystem". So I encourage people to put
>>> things there in the absence of any suitable subsystem, until someone
>>> gets enough motivation to start such a subsystem. This is more
>>> pragmatic than requesting subsystems to be created upfront.
>>>
>> Agreed.
>>
>> Note that there is already a pressure sensor in drivers/misc - bmp085.c.
>> That chip also includes a temperature sensor.
> 
> That driver proves exactly what I was trying to say about drivers/misc.
> Even though bmp085 has done everything correctly according to the
> rules (its interface is documented and it has gone into drivers/misc
> instead of another subsystem), we're now stuck with an interface that
> was written for a specific chip without widespread review, and need
> to apply it to all other similar drivers, or risk breaking applications.
> 
> The interface used in bmp085 has no way of finding out which devices
> are actually pressure/temparature sensors, other than by looking
> at the name of the attributes in every i2c device. There are probably
> ways to retrofit this without breaking compatibility, but the options
> are now fairly limited.
Agreed entirely, though that particular device actually has a reasonable
interface which bares a startling resemblence to hwmon / iio.
(I pushed back on that during review and Christoph was happy to
make merging later easy).

As an aside IIO very deliberately only makes type of sensor apparent in
via naming of sysfs attributes, as does hwmon. Obviously you'll
be looking in roughly the right place though so not quite so bad as
searching all possible buses though!
Any subgrouping doesn't really work as so many devices have fairly
random sets of sensors on them. (not sure if you were suggesting that
though!)
> 
>>> That being said, staging is another option nowadays.
>>>
>>>> While it seems that right now everyone is just trying to keep move
>>>> the driver to some other subsystem, I think it's worth noting that
>>>> it is indeed a useful thing to have the driver, I'm optimistic
>>>> that we can find some place for it. ;-)
>>>>
>>>> Now how about the IIO stuff? This is the first time I've even
>>>> heard about it. Does it have any major disadvantages besides
>>>> being staging-quality?
>>>
>>> This is indeed the major disadvantage. IIO seems to take a lot of time
>>> to move out of staging, although I don't know what the current ETA is.
>>>
>> In general it would be nice to have a "sensors" subsystem. iio is going into
>> that direction, so creating another one might not make much sense at this point.
> 
> It depends on the quality of the iio code base, which I have not looked
> at. If it's good enough, we could use that, and ideally find a way to
> be backwards-compatible with bmp085. Otherwise, creating a new subsystem
> could work as well, but only if the people behind iio support that move.
> A new subsystem of that sort would need to start out with a well-designed
> user interface, and allow moving over drivers from staging/iio without
> too much pain.
Interfaces (well sysfs ones anyway) is an area we have hammered pretty hard.
Arnd, if I you do have time to look at anything, take a look at the abi docs
in drivers/staging/iio/Documentation/ (mainly sysfs-bus-iio).  I've been
pushing others introducing similar drivers into misc/input to use these
unless they can come up with a good argument of why not.  Some fight back
on the basis we are only in staging so why should they take any notice.
Moves to agree these agreed across the kernel (mainly directed at
accelerometers as some of these make sense in input whereas others
definitely don't) didn't get very far sadly.  Right now Dmitry is
pretty much blocking any accelerometer sysfs interface purely because
he hasn't seen a consensus yet.

It took a lot of time to pin those interfaces down, so whilst we welcome
suggestions for changes/additions, they must keep generality if at all
possible.

Jonathan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add STMicroelectronics LPS001WP pressure sensor device driver into misc
  2011-03-15 11:11       ` [PATCH] Add STMicroelectronics LPS001WP pressure sensor device driver into misc Jonathan Cameron
@ 2011-03-15 12:51         ` Mark Brown
  2011-03-15 13:31           ` Arnd Bergmann
  2011-03-15 14:38           ` Jonathan Cameron
  2011-03-15 13:29         ` Arnd Bergmann
  1 sibling, 2 replies; 10+ messages in thread
From: Mark Brown @ 2011-03-15 12:51 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Arnd Bergmann, Jean Delvare, mems applications, rdunlap,
	carmine.iascone, matteo.dameno, rubini, linux-doc, linux-kernel,
	Guenter Roeck, Greg KH, linux-iio@vger.kernel.org

On Tue, Mar 15, 2011 at 11:11:00AM +0000, Jonathan Cameron wrote:
> On 03/15/11 09:38, Arnd Bergmann wrote:

[Reflowed Jonathan's text into 80 columns for legibility.]

> > Do you think it would help to split the iio codebase into a smaller part
> > for the relatively clean drivers that can be put into shape for
> > drivers/iio, and the bulk of the code that stays in staging for a bit
> > longer, until it gets converted to the new one in small chunks?

> 1) Spit functionality out in staging. This would give a core set that
> is basically the sysfs only stuff.  To do that we'd have to define a
> struct iio_dev_basic and make it an element of the iio_dev.  Prior to
> that we'd probably need to make pretty much all accesses into iio_dev
> via macros / inline functions which would not be a trivial
> undertaking.

> Then we could switch those drivers doing the minimum to the _basic
> form.  At that point we could perhaps attempt to move a couple of
> drivers and the abi docs out of staging.

> The disadvantages of this that come to mind are: * Makes the path to
> driver addition that I'd prefer trickier.  You write a basic sysfs
> only driver first, then add on stuff like events and buffering as
> separate patches. We could go the other way around like v4l2-subdev
> and have a base structure with the option of pointers to structures
> offering different combinations of features.  * Not many of the
> drivers I'd consider to be ready to go at the moment are actually in
> this _basic class. 

For what it's worth I have a few drivers I'd like to do which fall into
this category.  I've been put off working on them by the fact that I'm
not seeing a route out of staging for the subsystem.

> 2) Basically make a copy.  This would look like the original patch set did when we went

A third option is just to lift everything out of staging roughly as it
is now with anything that definitely needs redoing dropped, addressing
any review comments for mainline but not doing much else, and then
resume working on adding additional stuff.  It sounds like the userspace
interfaces that are there at present are mostly OK and most of the
issues are in-kernel?

You could perhaps have a Kconfig control for disabling any experimental
features in the API if you want to give people hints about areas that
are likely to churn.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add STMicroelectronics LPS001WP pressure sensor device driver into misc
  2011-03-15 11:11       ` [PATCH] Add STMicroelectronics LPS001WP pressure sensor device driver into misc Jonathan Cameron
  2011-03-15 12:51         ` Mark Brown
@ 2011-03-15 13:29         ` Arnd Bergmann
  2011-03-15 14:47           ` Jonathan Cameron
  1 sibling, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2011-03-15 13:29 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Jean Delvare, mems applications, rdunlap, carmine.iascone,
	matteo.dameno, rubini, linux-doc, linux-kernel, Guenter Roeck,
	Greg KH, linux-iio@vger.kernel.org

On Tuesday 15 March 2011, Jonathan Cameron wrote:
> On 03/15/11 09:38, Arnd Bergmann wrote:
> > Do you think it would help to split the iio codebase into a smaller part
> > for the relatively clean drivers that can be put into shape for
> > drivers/iio, and the bulk of the code that stays in staging for a bit
> > longer, until it gets converted to the new one in small chunks?
> > 
> > That may be less frustrating than trying to do it all at once. In particular,
> > you could do major internal rewrites much faster if you don't have
> > to immediately worry about breaking dozens of drivers in the process.
> > 
> Hi Arnd,
> 
> An interesting idea though I'm not entirely sure how it would
> work in practice.
> Two options occurred whilst cycling in this morning.
> 1) Spit functionality out in staging. This would give a core
> set that is basically the sysfs only stuff.  To do that we'd
> have to define a struct iio_dev_basic and make it an element
> of the iio_dev.  Prior to that we'd probably need to make pretty
> much all accesses into iio_dev via macros / inline functions 
> which would not be a trivial undertaking.

I think if you try to maintain compatibility between the
basic drivers and the complex stuff in the same tree, you
won't be able to gain much. Any major changes to address the TODO
items would still potentially impact all drivers.

> 2) Basically make a copy. 
> ...
> * Take out a subset of struct iio_dev (and associated functions
>   etc), initially just doing the 'bus' handling and sysfs direct
>   access to the devices.
> * Move over the relevant parts of Docs (mostly from sysfs-iio-bus)
> * Hack out the relevant chunks of a number of the cleaner drivers.
> (the driver and doc moves would occur in sets covering different
> types of devices to keep the review burden of looking at the 
> interface to a minimum. We could also clean up the remaining poorly
> defined abis as we do this (energy meters and resolvers IIRC).

Yes, this sounds like what I had in mind.

> Pause for a while (note that we would have to rigorously prevent
> any divergence between the two versions).

I wouldn't even worry about divergence at all. Just keep the
existing version alive and make sure that it doesn't conflict
with the new one. Any identifiers that are part of the user interface
could be renamed from iio to something like iioclassic to let
them coexist and to make sure that you don't have to start out
with iio2 for the non-staging version.

> * Copy over the events infrastructure
> * Move the next chunk of Docs
> * Lift over the events support from clean drivers.
> 
> Pause for another while
> 
> * Lift the core buffering support
> * Lift over the reset of the documentation.
> * Take over the hardware buffered device drivers
> * Take over kfifo wrapping code (has an inferior feature set
>   to ring_sw but easier to review by far).
> * synchronise the remains of our good driver set and kill
>   them off in staging.
> 
> That would leave the ugly drivers in staging to pull over
> as they get fixed up.

Yes. Or, with my variant, leave a copy of the core as well.

> Disadvantages of this:
> 
> * It's not necessarily trivial to break up complex drivers into
> the three elements described here. They have a tedious habit of
> interacting.  The issue would be that a lot of the code would seem
> like it has been done in a very strange way to any reviewer of 
> the parts they see at a time. Normally they get to look on to a
> later patch to see why stuff has been done as it has.  Here, would
> anyone really take a look at the driver in staging to understand 
> what is going on?

Then leave the complex drivers until you have the infrastructure
for them in the new core version.

> * The difficulty of keeping the stuff in staging in sync with
>   the stuff outside.
> 
> * Fiddly stuff to handle the fact we will for a while have two
>   drivers for an awful lot of stuff.

I think these would easily be avoided if you have two incompatible
subsystems. The disadvantage is that it would become harder to
move a driver from staging/iioclassic to drivers/iio when the
APIs are diverging.

	Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add STMicroelectronics LPS001WP pressure sensor device driver into misc
  2011-03-15 12:51         ` Mark Brown
@ 2011-03-15 13:31           ` Arnd Bergmann
  2011-03-15 14:35             ` Jonathan Cameron
  2011-03-15 14:38           ` Jonathan Cameron
  1 sibling, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2011-03-15 13:31 UTC (permalink / raw)
  To: Mark Brown
  Cc: Jonathan Cameron, Jean Delvare, mems applications, rdunlap,
	carmine.iascone, matteo.dameno, rubini, linux-doc, linux-kernel,
	Guenter Roeck, Greg KH, linux-iio@vger.kernel.org

On Tuesday 15 March 2011, Mark Brown wrote:
> > 2) Basically make a copy.  This would look like the original patch set did when we went
> 
> A third option is just to lift everything out of staging roughly as it
> is now with anything that definitely needs redoing dropped, addressing
> any review comments for mainline but not doing much else, and then
> resume working on adding additional stuff.  It sounds like the userspace
> interfaces that are there at present are mostly OK and most of the
> issues are in-kernel?
> 
> You could perhaps have a Kconfig control for disabling any experimental
> features in the API if you want to give people hints about areas that
> are likely to churn.

I agree, that sounds like a good idea for preparing the code before a split.

	Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add STMicroelectronics LPS001WP pressure sensor device driver into misc
  2011-03-15 11:30       ` Jonathan Cameron
@ 2011-03-15 13:58         ` Arnd Bergmann
  0 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2011-03-15 13:58 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Guenter Roeck, Jean Delvare, mems applications,
	rdunlap@xenotime.net, carmine.iascone@st.com,
	matteo.dameno@st.com, rubini@ipvvis.unipv.it,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-iio@vger.kernel.org

On Tuesday 15 March 2011, Jonathan Cameron wrote:
> Interfaces (well sysfs ones anyway) is an area we have hammered pretty hard.
> Arnd, if I you do have time to look at anything, take a look at the abi docs
> in drivers/staging/iio/Documentation/ (mainly sysfs-bus-iio).
 
Ok, I'll have a look.

	Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add STMicroelectronics LPS001WP pressure sensor device driver into misc
  2011-03-15 13:31           ` Arnd Bergmann
@ 2011-03-15 14:35             ` Jonathan Cameron
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2011-03-15 14:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Mark Brown, Jean Delvare, mems applications, rdunlap,
	carmine.iascone, matteo.dameno, rubini, linux-doc, linux-kernel,
	Guenter Roeck, Greg KH, linux-iio@vger.kernel.org

On 03/15/11 13:31, Arnd Bergmann wrote:
> On Tuesday 15 March 2011, Mark Brown wrote:
>>> 2) Basically make a copy.  This would look like the original patch set did when we went
>>
>> A third option is just to lift everything out of staging roughly as it
>> is now with anything that definitely needs redoing dropped, addressing
>> any review comments for mainline but not doing much else, and then
>> resume working on adding additional stuff.  It sounds like the userspace
>> interfaces that are there at present are mostly OK and most of the
>> issues are in-kernel?
>>
>> You could perhaps have a Kconfig control for disabling any experimental
>> features in the API if you want to give people hints about areas that
>> are likely to churn.
> 
> I agree, that sounds like a good idea for preparing the code before a split.
Indeed a good idea.  I'll add one of those.
> 
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add STMicroelectronics LPS001WP pressure sensor device driver into misc
  2011-03-15 12:51         ` Mark Brown
  2011-03-15 13:31           ` Arnd Bergmann
@ 2011-03-15 14:38           ` Jonathan Cameron
  1 sibling, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2011-03-15 14:38 UTC (permalink / raw)
  To: Mark Brown
  Cc: Arnd Bergmann, Jean Delvare, mems applications, rdunlap,
	carmine.iascone, matteo.dameno, rubini, linux-doc, linux-kernel,
	Guenter Roeck, Greg KH, linux-iio@vger.kernel.org

On 03/15/11 12:51, Mark Brown wrote:
> On Tue, Mar 15, 2011 at 11:11:00AM +0000, Jonathan Cameron wrote:
>> On 03/15/11 09:38, Arnd Bergmann wrote:
> 
> [Reflowed Jonathan's text into 80 columns for legibility.]
> 
>>> Do you think it would help to split the iio codebase into a smaller part
>>> for the relatively clean drivers that can be put into shape for
>>> drivers/iio, and the bulk of the code that stays in staging for a bit
>>> longer, until it gets converted to the new one in small chunks?
> 
>> 1) Spit functionality out in staging. This would give a core set that
>> is basically the sysfs only stuff.  To do that we'd have to define a
>> struct iio_dev_basic and make it an element of the iio_dev.  Prior to
>> that we'd probably need to make pretty much all accesses into iio_dev
>> via macros / inline functions which would not be a trivial
>> undertaking.
> 
>> Then we could switch those drivers doing the minimum to the _basic
>> form.  At that point we could perhaps attempt to move a couple of
>> drivers and the abi docs out of staging.
> 
>> The disadvantages of this that come to mind are: * Makes the path to
>> driver addition that I'd prefer trickier.  You write a basic sysfs
>> only driver first, then add on stuff like events and buffering as
>> separate patches. We could go the other way around like v4l2-subdev
>> and have a base structure with the option of pointers to structures
>> offering different combinations of features.  * Not many of the
>> drivers I'd consider to be ready to go at the moment are actually in
>> this _basic class. 
> 
> For what it's worth I have a few drivers I'd like to do which fall into
> this category.  I've been put off working on them by the fact that I'm
> not seeing a route out of staging for the subsystem.
> 
>> 2) Basically make a copy.  This would look like the original patch set did when we went
> 
> A third option is just to lift everything out of staging roughly as it
> is now with anything that definitely needs redoing dropped, addressing
> any review comments for mainline but not doing much else, and then
> resume working on adding additional stuff.  It sounds like the userspace
> interfaces that are there at present are mostly OK and most of the
> issues are in-kernel?
Mostly, though I suspect our events interface will cause some 'discussion'
and that sits one step above the absolute minimum driver.

Right now I want to push out the rewrite of the triggers, then I'll start
putting together a patch set to try and move some stuff over and see how
well things break up.




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add STMicroelectronics LPS001WP pressure sensor device driver into misc
  2011-03-15 13:29         ` Arnd Bergmann
@ 2011-03-15 14:47           ` Jonathan Cameron
  2011-03-15 16:50             ` Arnd Bergmann
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2011-03-15 14:47 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jean Delvare, mems applications, rdunlap, carmine.iascone,
	matteo.dameno, rubini, linux-doc, linux-kernel, Guenter Roeck,
	Greg KH, linux-iio@vger.kernel.org

On 03/15/11 13:29, Arnd Bergmann wrote:
> On Tuesday 15 March 2011, Jonathan Cameron wrote:
>> On 03/15/11 09:38, Arnd Bergmann wrote:
>>> Do you think it would help to split the iio codebase into a smaller part
>>> for the relatively clean drivers that can be put into shape for
>>> drivers/iio, and the bulk of the code that stays in staging for a bit
>>> longer, until it gets converted to the new one in small chunks?
>>>
>>> That may be less frustrating than trying to do it all at once. In particular,
>>> you could do major internal rewrites much faster if you don't have
>>> to immediately worry about breaking dozens of drivers in the process.
>>>
>> Hi Arnd,
>>
>> An interesting idea though I'm not entirely sure how it would
>> work in practice.
>> Two options occurred whilst cycling in this morning.
>> 1) Spit functionality out in staging. This would give a core
>> set that is basically the sysfs only stuff.  To do that we'd
>> have to define a struct iio_dev_basic and make it an element
>> of the iio_dev.  Prior to that we'd probably need to make pretty
>> much all accesses into iio_dev via macros / inline functions 
>> which would not be a trivial undertaking.
> 
> I think if you try to maintain compatibility between the
> basic drivers and the complex stuff in the same tree, you
> won't be able to gain much. Any major changes to address the TODO
> items would still potentially impact all drivers.
True though the todo's I know about shouldn't cause trouble for
the simple drivers.
> 
>> 2) Basically make a copy. 
>> ...
>> * Take out a subset of struct iio_dev (and associated functions
>>   etc), initially just doing the 'bus' handling and sysfs direct
>>   access to the devices.
>> * Move over the relevant parts of Docs (mostly from sysfs-iio-bus)
>> * Hack out the relevant chunks of a number of the cleaner drivers.
>> (the driver and doc moves would occur in sets covering different
>> types of devices to keep the review burden of looking at the 
>> interface to a minimum. We could also clean up the remaining poorly
>> defined abis as we do this (energy meters and resolvers IIRC).
> 
> Yes, this sounds like what I had in mind.
> 
>> Pause for a while (note that we would have to rigorously prevent
>> any divergence between the two versions).
> 
> I wouldn't even worry about divergence at all. Just keep the
> existing version alive and make sure that it doesn't conflict
> with the new one. Any identifiers that are part of the user interface
> could be renamed from iio to something like iioclassic to let
> them coexist and to make sure that you don't have to start out
> with iio2 for the non-staging version.
> 
>> * Copy over the events infrastructure
>> * Move the next chunk of Docs
>> * Lift over the events support from clean drivers.
>>
>> Pause for another while
>>
>> * Lift the core buffering support
>> * Lift over the reset of the documentation.
>> * Take over the hardware buffered device drivers
>> * Take over kfifo wrapping code (has an inferior feature set
>>   to ring_sw but easier to review by far).
>> * synchronise the remains of our good driver set and kill
>>   them off in staging.
>>
>> That would leave the ugly drivers in staging to pull over
>> as they get fixed up.
> 
> Yes. Or, with my variant, leave a copy of the core as well.
Sure, depending on what is left there.  If we have moved all of the
above across then there the non staging version should do everything
the staging version does..
> 
>> Disadvantages of this:
>>
>> * It's not necessarily trivial to break up complex drivers into
>> the three elements described here. They have a tedious habit of
>> interacting.  The issue would be that a lot of the code would seem
>> like it has been done in a very strange way to any reviewer of 
>> the parts they see at a time. Normally they get to look on to a
>> later patch to see why stuff has been done as it has.  Here, would
>> anyone really take a look at the driver in staging to understand 
>> what is going on?
> 
> Then leave the complex drivers until you have the infrastructure
> for them in the new core version.
Then the key thing is going to be convincing people that there is
a reason for a lot of what will go in early on, but they'll need to
look at staging to see why... I guess the version going into mainline
may need a lot of comments in the code.  Note for some whole classes
of devices there are only 'complex' drivers.
> 
>> * The difficulty of keeping the stuff in staging in sync with
>>   the stuff outside.
>>
>> * Fiddly stuff to handle the fact we will for a while have two
>>   drivers for an awful lot of stuff.
> 
> I think these would easily be avoided if you have two incompatible
> subsystems. The disadvantage is that it would become harder to
> move a driver from staging/iioclassic to drivers/iio when the
> APIs are diverging.
Will be interesting to see how bad this gets, but I guess one never
knows until one tries...

Thanks for your suggestions on this.  


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add STMicroelectronics LPS001WP pressure sensor device driver into misc
  2011-03-15 14:47           ` Jonathan Cameron
@ 2011-03-15 16:50             ` Arnd Bergmann
  0 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2011-03-15 16:50 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Jean Delvare, mems applications, rdunlap, carmine.iascone,
	matteo.dameno, rubini, linux-doc, linux-kernel, Guenter Roeck,
	Greg KH, linux-iio@vger.kernel.org

On Tuesday 15 March 2011, Jonathan Cameron wrote:
> On 03/15/11 13:29, Arnd Bergmann wrote:
> > On Tuesday 15 March 2011, Jonathan Cameron wrote:
> >> An interesting idea though I'm not entirely sure how it would
> >> work in practice.
> >> Two options occurred whilst cycling in this morning.
> >> 1) Spit functionality out in staging. This would give a core
> >> set that is basically the sysfs only stuff.  To do that we'd
> >> have to define a struct iio_dev_basic and make it an element
> >> of the iio_dev.  Prior to that we'd probably need to make pretty
> >> much all accesses into iio_dev via macros / inline functions 
> >> which would not be a trivial undertaking.
> > 
> > I think if you try to maintain compatibility between the
> > basic drivers and the complex stuff in the same tree, you
> > won't be able to gain much. Any major changes to address the TODO
> > items would still potentially impact all drivers.
>
> True though the todo's I know about shouldn't cause trouble for
> the simple drivers.

Ok. If that's the case, it may be simpler to just get the
core into shape for merging, and then do one driver at a
time, but leave all other drivers in staging.

However, without having looked at the code, I would still
assume that it's not that easy and you will actually break
drivers by fixing the core.

> >> That would leave the ugly drivers in staging to pull over
> >> as they get fixed up.
> > 
> > Yes. Or, with my variant, leave a copy of the core as well.
>
> Sure, depending on what is left there.  If we have moved all of the
> above across then there the non staging version should do everything
> the staging version does..

Yes. If they still have a compatible in-kernel API.
 
> > Then leave the complex drivers until you have the infrastructure
> > for them in the new core version.
> Then the key thing is going to be convincing people that there is
> a reason for a lot of what will go in early on, but they'll need to
> look at staging to see why... I guess the version going into mainline
> may need a lot of comments in the code.  Note for some whole classes
> of devices there are only 'complex' drivers.

Well, all the code is in the kernel in the staging drivers, so
any reviewer can look there to see how it's used. You can always
point to a specific driver in the changelog as an example when
a core component gets posted for review in order to have it
in the mainline tree.

	Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-03-15 16:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1300128906-1066-1-git-send-email-matteo.dameno@st.com>
     [not found] ` <20110314224244.3d6d23ba@endymion.delvare>
     [not found]   ` <4D7EA38F.5070002@cam.ac.uk>
     [not found]     ` <201103151038.40559.arnd@arndb.de>
2011-03-15 11:11       ` [PATCH] Add STMicroelectronics LPS001WP pressure sensor device driver into misc Jonathan Cameron
2011-03-15 12:51         ` Mark Brown
2011-03-15 13:31           ` Arnd Bergmann
2011-03-15 14:35             ` Jonathan Cameron
2011-03-15 14:38           ` Jonathan Cameron
2011-03-15 13:29         ` Arnd Bergmann
2011-03-15 14:47           ` Jonathan Cameron
2011-03-15 16:50             ` Arnd Bergmann
     [not found]   ` <20110314224859.GA16970@ericsson.com>
     [not found]     ` <201103151024.26436.arnd@arndb.de>
2011-03-15 11:30       ` Jonathan Cameron
2011-03-15 13:58         ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox