On Fri, 17 Jul 2009 16:17:56 +0200
Thomas Martitz <thomas.martitz_at_student.HTW-Berlin.de> wrote:

> Rafaël Carré schrieb:

> > In storage.h , storage_soft_reset and storage_disk_is_active were
> > already voided for some storage types so I thought it would be
> > right to continue in this direction.

> I don't agree with that. I think the opposite way would be proper, to
> enfore the API.

You can still use storage_sleep() in rockbox. The API hasn't changed.

It has changed in the plugins though, these functions are only present
when they are needed at all, conditionally on the player's hardware
(just like other functions conditionally present depending on
hardware's capabilities)

> >> The problem with this approach is how to expose those functions to
> >> the plugin API on single-driver, non-ATA targets. A solution for
> >> this might simply be to always use a concrete storage_* function.
> >> I can't really see that being much of an overhead.

> > They only really need to be exported when HAVE_DISK_STORAGE is
> > defined, right?

> See, here the "complexity is hidden for developers" argument becomes
> simply wrong. For core code it's hidden, for plugins not. That's even
> worse than #ifdeffing all the way IMO.

I don't think it is quite complex.

These functions in fact aren't related to the storage driver, but to
the physical storage.

I believe they should be removed from storage driver, and renamed to
disk_sleep / disk_spin etc to really reflect what they do and how they
should be used.

> That's the downside of a generic API. Is that a problem? Why was it
> API'fied in the first place?
> Either we use an API, which may cost a tiny bit binsize due to stubs,
> or we go for plain disk type specific functions.
>
> What we have now is a mix. An API which pretends to be one, but which
> isn't one actually. This is just bad, IMO.

I think these functions do not belong to this API, and in fact this
commit was not the correct way to go.

Instead they should be renamed (according to my above explanation)

gevaerts do you have an opinion, since you authored the refactoring of
this storage API ?

-- 
Rafaël Carré