2008-03-18 11:00:52

by Tomas Winkler

[permalink] [raw]
Subject: SDIO: IO-Ready Bit

I'm working on SDIO multi function device. One of the subdevices a
system device (SYS) who's purpose among the others is to initialize
the whole combo, mainly it's loads the firmware of the devices and
kick the sub devices. Each of the sub devices has it's own driver.
This dictates the order of the driver initialization and SYS device
has to compete it's work before other subdeivces drivers can access
the hardware. From hardware perspective device is ready when IO-Ready
bit in SDIO is set.
The first problem is that currently there is hard code timeout in
sdio_enable_func instead of using TPLFE_ENABLE_TIMEOUT_VAL
/*
* FIXME: This should timeout based on information in the CIS,
* but we don't have card to parse that yet.
*/
timeout = jiffies + HZ

This can be probably easily fixed. The significant issue is that this
is done in busy wait loop and that probe functions are called in
serially.

Since we cannot ensure that enumeration of SYS devices will be first
the other sub devices will fail in their probe functions while calling
sido_enable_func

One option is to move the sdio_enable_func to be called from a work
queue kicked from probe. This still requires non-busy wait timeout on
IO-Ready bit and we cannot fail probe func
if something goes wrong.
Second option would be somehow split the sdio probe function across
the enabled timeout.

Thanks
Tomas


2008-03-18 11:34:46

by Pierre Ossman

[permalink] [raw]
Subject: Re: SDIO: IO-Ready Bit

On Tue, 18 Mar 2008 13:00:34 +0200
"Tomas Winkler" <[email protected]> wrote:

> I'm working on SDIO multi function device. One of the subdevices a
> system device (SYS) who's purpose among the others is to initialize
> the whole combo, mainly it's loads the firmware of the devices and
> kick the sub devices. Each of the sub devices has it's own driver.
> This dictates the order of the driver initialization and SYS device
> has to compete it's work before other subdeivces drivers can access
> the hardware. From hardware perspective device is ready when IO-Ready
> bit in SDIO is set.

What a delightfully unfriendly design...

> The first problem is that currently there is hard code timeout in
> sdio_enable_func instead of using TPLFE_ENABLE_TIMEOUT_VAL
> /*
> * FIXME: This should timeout based on information in the CIS,
> * but we don't have card to parse that yet.
> */
> timeout = jiffies + HZ
>
> This can be probably easily fixed. The significant issue is that this
> is done in busy wait loop and that probe functions are called in
> serially.

The SDIO spec doesn't really allow such a card to exist. So it's not surprising that things break. Does any stack support this odd-ball card?

>
> Since we cannot ensure that enumeration of SYS devices will be first
> the other sub devices will fail in their probe functions while calling
> sido_enable_func
>
> One option is to move the sdio_enable_func to be called from a work
> queue kicked from probe. This still requires non-busy wait timeout on
> IO-Ready bit and we cannot fail probe func
> if something goes wrong.
> Second option would be somehow split the sdio probe function across
> the enabled timeout.

I think it's a bit more complex than that. There are likely locks all over the place, both in the relevant driver and the driver model core, that makes this very difficult. Not to mention the fact that the correct timeout will be finite, and you never know how long it will take you to load the firmware.

A better approach is to simply have the drivers for your subfunctions synchronize things. Make sure noone calls sdio_enable_func() until the firmware has been loaded. This could even be done from user space in case you want to use a standard driver.

Rgds
Pierre


Attachments:
signature.asc (197.00 B)

2008-03-19 22:11:53

by Pierre Ossman

[permalink] [raw]
Subject: Re: SDIO: IO-Ready Bit

On Tue, 18 Mar 2008 19:43:40 +0200
"Tomas Winkler" <[email protected]> wrote:

> On Tue, Mar 18, 2008 at 1:33 PM, Pierre Ossman <[email protected]> wrote:
> >
> > A better approach is to simply have the drivers for your subfunctions synchronize things. Make sure noone calls sdio_enable_func() until the firmware has been loaded.
>
> This could even be done from user space in case you want to use a
> standard driver.
>
> Won't this prevent us from compiling the drivers into kernel?
> Second how this possible especially with standard driver as they are
> loaded. I know you can enforce module loading but how can you enforce
> probing order form user space?

You can tell the kernel to not automatically attach drivers to devices.
That way you can have a user space process control exactly the order
and timing of things. I haven't played around with this feature myself
though, so I can't really provide you with any good information on how
to use it. You should be able to google up something useful.

Rgds
Pierre


Attachments:
signature.asc (197.00 B)

2008-03-19 22:23:38

by Tomas Winkler

[permalink] [raw]
Subject: Re: SDIO: IO-Ready Bit

On Tue, Mar 18, 2008 at 1:33 PM, Pierre Ossman <[email protected]> wrote:
> On Tue, 18 Mar 2008 13:00:34 +0200
> "Tomas Winkler" <[email protected]> wrote:
>
> > I'm working on SDIO multi function device. One of the subdevices a
> > system device (SYS) who's purpose among the others is to initialize
> > the whole combo, mainly it's loads the firmware of the devices and
> > kick the sub devices. Each of the sub devices has it's own driver.
> > This dictates the order of the driver initialization and SYS device
> > has to compete it's work before other subdeivces drivers can access
> > the hardware. From hardware perspective device is ready when IO-Ready
> > bit in SDIO is set.
>
> What a delightfully unfriendly design...

Yes, but it's damage is already done.

>
> > The first problem is that currently there is hard code timeout in
> > sdio_enable_func instead of using TPLFE_ENABLE_TIMEOUT_VAL
> > /*
> > * FIXME: This should timeout based on information in the CIS,
> > * but we don't have card to parse that yet.
> > */
> > timeout = jiffies + HZ
> >
> > This can be probably easily fixed. The significant issue is that this
> > is done in busy wait loop and that probe functions are called in
> > serially.
>
> The SDIO spec doesn't really allow such a card to exist. So it's not surprising that things break.

Not sure to which part of the above paragraph you refer here. The
timeout is defined by spec.

> Does any stack support this odd-ball card?

>
>
> >
> > Since we cannot ensure that enumeration of SYS devices will be first
> > the other sub devices will fail in their probe functions while calling
> > sido_enable_func
> >
> > One option is to move the sdio_enable_func to be called from a work
> > queue kicked from probe. This still requires non-busy wait timeout on
> > IO-Ready bit and we cannot fail probe func
> > if something goes wrong.
> > Second option would be somehow split the sdio probe function across
> > the enabled timeout.
>
> I think it's a bit more complex than that. There are likely locks all over the place, both in the relevant driver and the driver model core, that makes this very difficult. Not to mention the fact that the correct timeout will be finite, and you never know how long it will take you to load the firmware.

Good point but IMHO the timeout can got to minutes so it's converging
to infinite in this case.



>
> A better approach is to simply have the drivers for your subfunctions synchronize things. Make sure noone calls sdio_enable_func() until the firmware has been loaded.

This could even be done from user space in case you want to use a
standard driver.

Won't this prevent us from compiling the drivers into kernel?
Second how this possible especially with standard driver as they are
loaded. I know you can enforce module loading but how can you enforce
probing order form user space?

Thanks
Tomas

> Rgds
> Pierre
>
>

2008-03-22 22:10:31

by Tomas Winkler

[permalink] [raw]
Subject: Re: SDIO: IO-Ready Bit

On Tue, Mar 18, 2008 at 8:20 PM, Pierre Ossman <[email protected]> wrote:
> On Tue, 18 Mar 2008 19:43:40 +0200
>
> "Tomas Winkler" <[email protected]> wrote:
>
>
> > On Tue, Mar 18, 2008 at 1:33 PM, Pierre Ossman <[email protected]> wrote:
> > >
>
> > > A better approach is to simply have the drivers for your subfunctions synchronize things. Make sure noone calls sdio_enable_func() until the firmware has been loaded.
> >
> > This could even be done from user space in case you want to use a
> > standard driver.
> >
> > Won't this prevent us from compiling the drivers into kernel?
> > Second how this possible especially with standard driver as they are
> > loaded. I know you can enforce module loading but how can you enforce
> > probing order form user space?
>
> You can tell the kernel to not automatically attach drivers to devices.
> That way you can have a user space process control exactly the order
> and timing of things. I haven't played around with this feature myself
> though, so I can't really provide you with any good information on how
> to use it. You should be able to google up something useful.

I couldn't find anything that can prevent the probe function to be
called in case the module is already loaded. I've looked at udev and
HAL and no much I found.
I will dig into kernel sources, maybe finding out how can be bus
enumeration triggered again or something like this.
Any pointers will be appreciated.

Thanks
Tomas

2008-03-24 12:54:15

by Pierre Ossman

[permalink] [raw]
Subject: Re: SDIO: IO-Ready Bit

On Sun, 23 Mar 2008 00:10:19 +0200
"Tomas Winkler" <[email protected]> wrote:

>
> I couldn't find anything that can prevent the probe function to be
> called in case the module is already loaded. I've looked at udev and
> HAL and no much I found.
> I will dig into kernel sources, maybe finding out how can be bus
> enumeration triggered again or something like this.
> Any pointers will be appreciated.
>

Here's the patch that added the functionality:

http://lkml.org/lkml/2007/4/27/466

Rgds
Pierre


Attachments:
signature.asc (197.00 B)

2008-03-24 14:41:36

by Tomas Winkler

[permalink] [raw]
Subject: Re: SDIO: IO-Ready Bit

On Mon, Mar 24, 2008 at 3:53 PM, Pierre Ossman <[email protected]> wrote:
> On Sun, 23 Mar 2008 00:10:19 +0200
>
> "Tomas Winkler" <[email protected]> wrote:
>
> >
>
> > I couldn't find anything that can prevent the probe function to be
> > called in case the module is already loaded. I've looked at udev and
> > HAL and no much I found.
> > I will dig into kernel sources, maybe finding out how can be bus
> > enumeration triggered again or something like this.
> > Any pointers will be appreciated.
> >
>
> Here's the patch that added the functionality:
>
> http://lkml.org/lkml/2007/4/27/466


Thanks a lot. I will look at this and how it can be used.

Anyhow I've seen that concept of parallel probe is not so new. It was
in 2.6.18 and removed in 2.619.
Probably different approach, maybe more would be adding a parallel
initial function.
Probe will just register to the bus and relevant subsystem, it will be fast.
Optional Initialization function will run after probe in different
thread, this would be the place for running FW download.
Device won't be considered initialized till this function haven't
completed or is not implemented.


Any comments are welcome.

Thanks
Tomas



> Rgds
> Pierre
>

2008-03-29 08:05:28

by Pierre Ossman

[permalink] [raw]
Subject: Re: SDIO: IO-Ready Bit

On Mon, 24 Mar 2008 16:41:25 +0200
"Tomas Winkler" <[email protected]> wrote:

>
> Anyhow I've seen that concept of parallel probe is not so new. It was
> in 2.6.18 and removed in 2.619.
> Probably different approach, maybe more would be adding a parallel
> initial function.
> Probe will just register to the bus and relevant subsystem, it will be fast.
> Optional Initialization function will run after probe in different
> thread, this would be the place for running FW download.
> Device won't be considered initialized till this function haven't
> completed or is not implemented.
>

I would suspect that would just slow things down in most cases. Most
busses have shared bandwidth, so letting each device take turns is
usually faster than a thundering herd.

Feel free to make your case to the device core maintainers though.

Rgds
Pierre


Attachments:
signature.asc (197.00 B)