2012-06-13 10:41:10

by Wei Ni

[permalink] [raw]
Subject: Where to power on the wifi device before loading the driver.

Hi, all
I'm working on the tegra30 wifi upstream issue.

The tegra30 board (Cardhu) use Broadcom 4329 as wifi device, and use brcmfmac as the wifi driver.

In the brcmfmac init routine, it call sdio_register_driver() to register driver, if the wifi device is powered on, then the mmc driver will enumerate it, and call the probe callback routine.

On the Cardhu, the wifi's power is controlled by two gpios (power-gpio and reset-gpio), the default state is power-off. So we need to power on it before calling sdio_register_driver(), if not, the mmc driver can't enumerate it, and will not call the probe routine.
This power on sequence is:
set power-gpio to 1 ;
mdelay(100) ;
set reset-gpio to 1 ;
mdelay(200);

My question is where to power on the wifi. We may have three places to power on it:
1. power on it in the brcmfmac driver before calling sdio_register_driver(). But I think this power sequence is special for tegra30 cardhu, it's not good to add it in the generic wifi driver, because different board may use the different way to power on the wifi.
2. power on it in the mmc driver. In our tegra SD driver, it has power-gpios property, which allow the slot to be powered. But this power is for mmc slot, could we add this wifi power sequence in the tegra SD driver?
3. hard-coded into DT. Set these gpios in the DT, something like pinmux settings, but in this way, it's not good to put the mdelay() value in the DT.

I have no good idea for it, does anyone has suggestion?

Thanks.
Wei.

---
nvpublic





2012-06-19 09:17:22

by Mark Brown

[permalink] [raw]
Subject: Re: Where to power on the wifi device before loading the driver.

On Tue, Jun 19, 2012 at 12:25:58PM +0800, Wei Ni wrote:

As Stephen previously said please fix your mail formatting - word
wrapping within paragraphs is important!

> On Tue, Jun 19, 2012 at 09:23:35, Philip Rakity wrote:
> >Why can't you use the regulator notify to get called back on power or
> >voltage change options to plumb in the manipulation of the gpio. I can
> >imagine you might need to add a notify call in core.c if you need to
> >assert the gpio before power is applied
>
> Yes, we can use the regulator notify, but the Tegra30 support is via
> device tree, I think there have no special board file to run the call
> back.

You can mix board files with device tree for cases where device tree is
not yet up to the job, though the goal should be to come up with a way
of expressing things in device tree.


Attachments:
(No filename) (822.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-06-18 15:01:49

by Stephen Warren

[permalink] [raw]
Subject: Re: Where to power on the wifi device before loading the driver.

On 06/18/2012 02:03 AM, Laxman Dewangan wrote:
> Rakesh Kumar wrote at Monday, June 18, 2012 1:11 PM:
> > Stephen Warren wrote:
> >> Now if this means adding a child node under the SDIO controller
> >> to represent the attached device, and storing any settings
> >> required by that device in that child node, that's probably a
> >> reasonable basic approach.
> >>
> >> BTW, which GPIO is the power GPIO; is it WF_EN on the schematic?
> >> That seems reasonable to represent as a GPIO rather than a
> >> regulator since it connects directly into the WiFi device as a
> >> GPIO, and its use within the WiFi device can indeed be governed
> >> purely internally to the WiFi driver/HW. However, if this is some
> >> GPIO that controls the power to e.g. VBAT3V3_IN_WF, VDDIO_WF, or
> >> other power supply to the WiFi card, then it'd be better
> >> represented as a regulator, since the control point is outside the
> >> WiFi device.
> >
> > Tegra uses two GPIO (WF_EN and WF_RST) to power on and reset bcm4329
> > card. In case of bcm4329, these two lines are shorted. Tegra does
> > not control VBAT3V3_IN_WF, VDDIO_WF, or other power supply to the
> > WiFi card based on these GPIO. Uses of these GPIO are internal to
> > WiFi HW. It is reasonable to represent as a GPU rather than
> > regulator.
>
> If it is for power then it has to go via regulator. It does not make
> sense to directly control the gpio inside the wifi driver.

As far as the board goes, WF_EN is just a GPIO signal to the WiFi card;
it doesn't gate power to the card. If it gates power inside the card,
that's an internal implementation detail of the card, and something I'd
be fine with the driver knowing directly, and hence I'm OK with
representing this as a GPIO rather than a regulator - that's what it is
externally to the WiFi device.

(BTW everyone, many of the emails in this thread are awfully formatted -
top-posted and not word-wrapped. It's very hard to read them... I tried
to reformat everything and fix it in this email)

2012-06-14 11:28:11

by Wei Ni

[permalink] [raw]
Subject: RE: Where to power on the wifi device before loading the driver.

On Thu, Jun 14, 2012 at 05:17:06, Stephen Warren wrote:
>On 06/13/2012 04:40 AM, Wei Ni wrote:
>> Hi, all
>> I'm working on the tegra30 wifi upstream issue.
>>
>> The tegra30 board (Cardhu) use Broadcom 4329 as wifi device, and use brcmfmac as the wifi driver.
>>
>> In the brcmfmac init routine, it call sdio_register_driver() to register driver, if the wifi device is powered on, then the mmc driver will enumerate it, and call the probe callback routine.
>>
>> On the Cardhu, the wifi's power is controlled by two gpios
>> (power-gpio and reset-gpio), the default state is power-off. So we need to power on it before calling sdio_register_driver(), if not, the mmc driver can't enumerate it, and will not call the probe routine.
>> This power on sequence is:
>> set power-gpio to 1 ;
>> mdelay(100) ;
>> set reset-gpio to 1 ;
>> mdelay(200);
>>
>> My question is where to power on the wifi. We may have three places to power on it:
>> 1. power on it in the brcmfmac driver before calling
>> sdio_register_driver(). But I think this power sequence is special for tegra30 cardhu, it's not good to add it in the generic wifi driver, because different board may use the different way to power on the wifi.
>> 2. power on it in the mmc driver. In our tegra SD driver, it has
>> power-gpios property, which allow the slot to be powered. But this power is for mmc slot, could we add this wifi power sequence in the tegra SD driver?
>> 3. hard-coded into DT. Set these gpios in the DT, something like pinmux settings, but in this way, it's not good to put the mdelay() value in the DT.
>>
>> I have no good idea for it, does anyone has suggestion?
>
>The core of the issue is that:
>
>* Tegra30 support is via device tree.
>* We have an SDIO bus, and the WiFi device attached to that bus is enumerable.
>* Since the WiFi device is enumerable, no node exists in the DT to represent it.
>* However, the driver for the WiFi device needs certain information,
>such as the reset GPIO ID and perhaps power GPIO.
>
>For the power GPIO, it seems reasonable to either use the existing
>Tegra SD controller's power-gpios DT property, or replace that property
>with a real regulator binding. The SD driver would then control this.
>Still, that approach would mean the WiFi driver wouldn't be able to control power to the device directly, which might not be a good thing.
>
>However, I'm not sure that the reset GPIO is also something that should
>be controlled by the SD card driver; it seems like it's much more closely related to the WiFi device/driver.
>
>I wonder if the power and reset GPIO shouldn't be represented as a
>combined custom regulator type, which knows how to sequence multiple GPIOs.
>
>Perhaps SDIO "client" devices also need a way to communicate with their
>"host port" to obtain services such as power and reset control?
>
>This is all very similar to the WiFi rfkill discussion we have re: the
>Toshiba AC100 a little while back, although that was USB rather than SDIO.

In the rfkill-gpio, it already have the reset-gpio and shutdown-gpio.
I test it, add a device node in the DT to pass the gpio properties.
It will use these gpios to power up/off , but its default power sequence is different with the 4329.
It block==0, it mean power up, it will set reset-gpio first, then set power-gpio next.
I think it should set power-gpio first, is it right?



2012-06-13 17:33:18

by Franky Lin

[permalink] [raw]
Subject: Re: Where to power on the wifi device before loading the driver.

Hi Wei,

On 06/13/2012 03:40 AM, Wei Ni wrote:
> In the brcmfmac init routine, it call sdio_register_driver() to register driver, if the wifi device is powered on, then the mmc driver will enumerate it, and call the probe callback routine.
>
> On the Cardhu, the wifi's power is controlled by two gpios (power-gpio and reset-gpio), the default state is power-off. So we need to power on it before calling sdio_register_driver(), if not, the mmc driver can't enumerate it, and will not call the probe routine.
> This power on sequence is:
> set power-gpio to 1 ;
> mdelay(100) ;
> set reset-gpio to 1 ;
> mdelay(200);
>
> My question is where to power on the wifi. We may have three places to power on it:
> 1. power on it in the brcmfmac driver before calling sdio_register_driver(). But I think this power sequence is special for tegra30 cardhu, it's not good to add it in the generic wifi driver, because different board may use the different way to power on the wifi.
> 2. power on it in the mmc driver. In our tegra SD driver, it has power-gpios property, which allow the slot to be powered. But this power is for mmc slot, could we add this wifi power sequence in the tegra SD driver?
> 3. hard-coded into DT. Set these gpios in the DT, something like pinmux settings, but in this way, it's not good to put the mdelay() value in the DT.

We introduced oob interrupt support in 3.5 [1]. We are using a virtual
platform device to retrieve board specific oob interrupt GPIO setting.
You should be able to implement the power control in this way as well.
Brcmfmac gets the GPIOs through platform device interface, powers up the
chip and triggers a card detection. Then 4329 should be enumerated by
MMC stack. The reason we didn't implement this is the card detection.
Some design doesn't have hardware card detection since the WiFi chip is
already on board. And the current MMC stack doesn't have virtual card
detection interface.

Franky

[1]: http://www.spinics.net/lists/linux-wireless/msg89335.html


2012-06-14 06:31:36

by Thierry Reding

[permalink] [raw]
Subject: Re: Where to power on the wifi device before loading the driver.

On Wed, Jun 13, 2012 at 03:17:06PM -0600, Stephen Warren wrote:
> On 06/13/2012 04:40 AM, Wei Ni wrote:
> > Hi, all
> > I'm working on the tegra30 wifi upstream issue.
> >
> > The tegra30 board (Cardhu) use Broadcom 4329 as wifi device, and use brcmfmac as the wifi driver.
> >
> > In the brcmfmac init routine, it call sdio_register_driver() to register driver, if the wifi device is powered on, then the mmc driver will enumerate it, and call the probe callback routine.
> >
> > On the Cardhu, the wifi's power is controlled by two gpios (power-gpio and reset-gpio), the default state is power-off. So we need to power on it before calling sdio_register_driver(), if not, the mmc driver can't enumerate it, and will not call the probe routine.
> > This power on sequence is:
> > set power-gpio to 1 ;
> > mdelay(100) ;
> > set reset-gpio to 1 ;
> > mdelay(200);
> >
> > My question is where to power on the wifi. We may have three places to power on it:
> > 1. power on it in the brcmfmac driver before calling sdio_register_driver(). But I think this power sequence is special for tegra30 cardhu, it's not good to add it in the generic wifi driver, because different board may use the different way to power on the wifi.
> > 2. power on it in the mmc driver. In our tegra SD driver, it has power-gpios property, which allow the slot to be powered. But this power is for mmc slot, could we add this wifi power sequence in the tegra SD driver?
> > 3. hard-coded into DT. Set these gpios in the DT, something like pinmux settings, but in this way, it's not good to put the mdelay() value in the DT.
> >
> > I have no good idea for it, does anyone has suggestion?
>
> The core of the issue is that:
>
> * Tegra30 support is via device tree.
> * We have an SDIO bus, and the WiFi device attached to that bus is
> enumerable.
> * Since the WiFi device is enumerable, no node exists in the DT to
> represent it.
> * However, the driver for the WiFi device needs certain information,
> such as the reset GPIO ID and perhaps power GPIO.

PCI devices are also enumerable and yet they can be matched up with nodes in
the device tree. Perhaps something similar could be added for the SDIO bus?

Thierry


Attachments:
(No filename) (2.15 kB)
(No filename) (198.00 B)
Download all attachments

2012-06-15 15:49:14

by Stephen Warren

[permalink] [raw]
Subject: Re: Where to power on the wifi device before loading the driver.

On 06/15/2012 12:09 AM, Wei Ni wrote:
> On Thu, Jun 14, 2012 at 23:54:22, Stephen Warren wrote:
>>>>> The core of the issue is that:
>>>
>>>>> * Tegra30 support is via device tree. * We have an SDIO bus, and
>>>>> the WiFi device attached to that bus is enumerable. * Since the
>>>>> WiFi device is enumerable, no node exists in the DT to represent
>>>>> it. * However, the driver for the WiFi device needs certain
>>>>> information, such as the reset GPIO ID and perhaps power GPIO.
>>>
>>>> PCI devices are also enumerable and yet they can be matched up with
>>>> nodes in the device tree. Perhaps something similar could be added
>>>> for the SDIO bus?
>>>
>>> This seems to make the most sense - pushing this through the
>>> regulator API is just a bodge.
>>
>> Yes, that seems reasonable.
>>
>> Presumably the power GPIO should be a fixed regulator though, since it
>> is a power control not just a plain old GPIO? That said, the current driver apparently deals with this as a GPIO already.
>>
>> The reset GPIO can separately/directly controlled by the WiFi driver though.
>
> I talked with Franky, this power sequence is generally for 4329, so it mean this sequence can be put into the wifi driver.
> We can use the virtual platform device both for OOB and non OOB.
> I will send out patches later.

Can you please expand on what a "virtual platform device" is; device
tree typically represents real hardware rather than anything "virtual".

Now if this means adding a child node under the SDIO controller to
represent the attached device, and storing any settings required by that
device in that child node, that's probably a reasonable basic approach.

BTW, which GPIO is the power GPIO; is it WF_EN on the schematic? That
seems reasonable to represent as a GPIO rather than a regulator since it
connects directly into the WiFi device as a GPIO, and its use within the
WiFi device can indeed be governed purely internally to the WiFi
driver/HW. However, if this is some GPIO that controls the power to e.g.
VBAT3V3_IN_WF, VDDIO_WF, or other power supply to the WiFi card, then
it'd be better represented as a regulator, since the control point is
outside the WiFi device.

2012-06-15 06:10:11

by Wei Ni

[permalink] [raw]
Subject: RE: Where to power on the wifi device before loading the driver.

On Thu, Jun 14, 2012 at 23:54:22, Stephen Warren wrote:
>>>> The core of the issue is that:
>>
>>>> * Tegra30 support is via device tree. * We have an SDIO bus, and
>>>> the WiFi device attached to that bus is enumerable. * Since the
>>>> WiFi device is enumerable, no node exists in the DT to represent
>>>> it. * However, the driver for the WiFi device needs certain
>>>> information, such as the reset GPIO ID and perhaps power GPIO.
>>
>>> PCI devices are also enumerable and yet they can be matched up with
>>> nodes in the device tree. Perhaps something similar could be added
>>> for the SDIO bus?
>>
>> This seems to make the most sense - pushing this through the
>> regulator API is just a bodge.
>
>Yes, that seems reasonable.
>
>Presumably the power GPIO should be a fixed regulator though, since it
>is a power control not just a plain old GPIO? That said, the current driver apparently deals with this as a GPIO already.
>
>The reset GPIO can separately/directly controlled by the WiFi driver though.

I talked with Franky, this power sequence is generally for 4329, so it mean this sequence can be put into the wifi driver.
We can use the virtual platform device both for OOB and non OOB.
I will send out patches later.

---
nvpublic

2012-06-20 10:47:46

by Mark Brown

[permalink] [raw]
Subject: Re: Where to power on the wifi device before loading the driver.

On Tue, Jun 19, 2012 at 06:01:54PM -0600, Stephen Warren wrote:

> Unfortunately, it should be noted that the WiFi device socket on Cardhu
> apparently isn't some standardized thing (like PCIe) but something
> rather more custom, although there are apparently devices available to
> plug into it from multiple module vendors which contain WiFi chips from
> multiple chip vendors.

Right, and indeed someone may decide to plug in something that isn't a
WiFi device at all but some other SDIO attached device with totally
different uses for those GPIOs. Really these boards are an example of
the sort of thing I was talking about with modular reference systems,
though a simpler case as given what's brought out you probably can just
do something like list all the options that are there and allow hotplug
to sort things out.


Attachments:
(No filename) (825.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-06-20 00:01:59

by Stephen Warren

[permalink] [raw]
Subject: Re: Where to power on the wifi device before loading the driver.

On 06/19/2012 03:44 AM, Wei Ni wrote:
> On Tue, Jun 19, 2012 at 17:17:19, Mark Brown wrote:
>> * PGP Signed by an unknown key
>>
>> On Tue, Jun 19, 2012 at 12:25:58PM +0800, Wei Ni wrote:
>>
>> As Stephen previously said please fix your mail formatting - word
>> wrapping within paragraphs is important!
>>
>>> On Tue, Jun 19, 2012 at 09:23:35, Philip Rakity wrote:
>>>> Why can't you use the regulator notify to get called back on power
>>>> or voltage change options to plumb in the manipulation of the gpio.
>>>> I can imagine you might need to add a notify call in core.c if you
>>>> need to assert the gpio before power is applied
>>>
>>> Yes, we can use the regulator notify, but the Tegra30 support is via
>>> device tree, I think there have no special board file to run the call
>>> back.
>>
>> You can mix board files with device tree for cases where device tree is
>> not yet up to the job, though the goal should be to come up with a way of expressing things in device tree.
>
> I think we wish to use the device tree to handle these gpios, pass them to the related driver to assert them.
> If we mix the board files, we even can set these gpio directly, not use the notify. But I think this is not a goog way.

Yes, I would definitely not want to add this to a board file; there is
no board file for Cardhu, and we're getting close to deleting all the
board files that are left for other boards.

I think what we need is some way of matching a DT node to a device even
when that device was instantiated through some probing mechanism such as
SDIO or USB (and I've heard hints that one can already do this for PCI;
I should investigate).

So, we start off with the plain SDHCI controller node:

sdhci@78000000 {
compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
reg = <0x78000000 0x200>;
interrupts = <0 14 0x04>;
};

Then, we add a child node to represent the device that's attached:

sdhci@78000000 {
compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
reg = <0x78000000 0x200>;
interrupts = <0 14 0x04>;

sdio-device {
reset-gpios = <...>;
enable-gpios = <...>;
};
};

When the SDHCI controller/core instantiates the child device it finds on
the HW bus, it initializes that struct device's of_node with a pointer
to the sdio-device node above. From there, the child device can get DT
properties such as GPIOs in the usual way.

However, there are still some issues that need thought here; what if (as
is I believe the case on Cardhu) we can in fact plug in multiple
different types of device into the socket? Might we end up with
something like:

sdhci@78000000 {
compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
reg = <0x78000000 0x200>;
interrupts = <0 14 0x04>;

sdio-device-option@0 {
compatible = "broadcom,bcm4239";
reset-gpios = <...>;
enable-gpios = <...>;
};

sdio-device-option@1 {
compatible = "broadcom,bcm4330";
reset-gpios = <...>;
enable-gpios = <...>;
};
};

and match on compatible value?

But in this case, at least some of the data those two drivers want from
that node is the same; the 2 GPIOs that control the reset and enable
signals. Do we just make the bindings for those two devices happen to
share the same properties so that we can get away with just one node
irrespective of what device type has been found? What we're really
modeling is the card slot, and the services it provides, so perhaps we
should really end up with:

sdhci@78000000 {
compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
reg = <0x78000000 0x200>;
interrupts = <0 14 0x04>;

socket {
reset-gpios = <...>;
enable-gpios = <...>;

sdio-device-option@0 {
compatible = "broadcom,bcm4239";
};

sdio-device-option@1 {
compatible = "broadcom,bcm4330";
};
};
};

Where somehow the WiFi drivers can obtain the reset/enable service from
the socket that hosts them, but still have nodes for the attached device
itself in case any additional driver-specific configuration is needed.

That sounds somewhat similar to the virtual platform device that was
mentioned earlier in this thread by Broadcom, although I'm not sure
whether explicitly modeling it as a platform device makes sense, rather
than the socket providing services to the WiFi module, not necessarily
through a device.

Unfortunately, it should be noted that the WiFi device socket on Cardhu
apparently isn't some standardized thing (like PCIe) but something
rather more custom, although there are apparently devices available to
plug into it from multiple module vendors which contain WiFi chips from
multiple chip vendors.

Anyway, these are just some quick thoughts on the topic - obviously more
flesh is needed.

2012-06-20 16:51:47

by Stephen Warren

[permalink] [raw]
Subject: Re: Where to power on the wifi device before loading the driver.

On 06/20/2012 05:28 AM, Wei Ni wrote:
...
> Stephen, as Rakesh said that in case of bcm4329, the two GPIO (WF_EN
> and WF_RST) are shorted. So I think we can simply to use the
> power-gpios properity in the SD driver, just use one gpio I test it
> in this way, it runs ok, the bcm4329 can be powered up. We can use
> this option.

Wei, again, your emails and not correctly word-wrapped to well under 80
columns. I know it's a pain to do that with Exchange/Outlook, but it's
really difficult for anyone to read emails that aren't correctly formatted.

So yes, once we'd resolved this general issue, that was going to be my
next topic of discussion.

I looked at the schematic of the 4329 module, and the GPIOs aren't
shorted, but rather there's a stuffing option - either WF_EN or WF_RST
is actually connected to the 4329's WL_SHUTDOWN_N_RST pin. I believe
WF_EN is the one connected. If the two signals were shorted, you'd need
to control both of them to avoid sinking power from one of Tegra's GPIO
outputs into the other if they were set to differing values.

So, yes, using the power-gpios property will probably work in this case.

This rather begs the question: WTF is our downstream kernel doing
actively controlling both GPIOs with time delays between each? Do you
have the schematic for the 4330 module, or indeed any other module
that's used on Cardhu? If so, please do send them to me internally.
While this workaround might work for the 4329, additional research would
be useful so we can determine whether we still have a problem to solve
for other modules.

Thanks.

2012-06-19 04:26:08

by Wei Ni

[permalink] [raw]
Subject: RE: Where to power on the wifi device before loading the driver.

On Tue, Jun 19, 2012 at 09:23:35, Philip Rakity wrote:
>Why can't you use the regulator notify to get called back on power or
>voltage change options to plumb in the manipulation of the gpio. I can
>imagine you might need to add a notify call in core.c if you need to
>assert the gpio before power is applied

Yes, we can use the regulator notify, but the Tegra30 support is via device tree, I think there have no special board file to run the call back.

>
>
>________________________________________
>From: [email protected] [[email protected]]
>On Behalf Of Wei Ni [[email protected]]
>Sent: Sunday, June 17, 2012 11:20 PM
>To: 'Stephen Warren'; Rakesh Kumar
>Cc: Mark Brown; '[email protected]'; Thierry Reding; Mursalin Akon;
>'[email protected]'; [email protected];
>'[email protected]'; [email protected];
>[email protected]
>Subject: RE: Where to power on the wifi device before loading the driver.
>
>On Fri, Jun 15, 2012 at 23:49:10, Stephen Warren wrote:
>>> I talked with Franky, this power sequence is generally for 4329, so
>>> it mean this sequence can be put into the wifi driver.
>>> We can use the virtual platform device both for OOB and non OOB.
>>> I will send out patches later.
>>
>>Can you please expand on what a "virtual platform device" is; device
>>tree typically represents real
>hardware rather than anything "virtual".
>
>The "virtual platform device" is created before the real wifi device,
>so that it can pass the gpios to the driver, and power up the device before calling sdio_register_driver.
>But as Franky said, this power sequence is only generally for 4329, not
>for the USB dongle chips. So it's not a good idea to add this "power up" in brcmfmac.
>
>>
>>Now if this means adding a child node under the SDIO controller to
>>represent the attached device, and
>storing any settings required by that device in that child node, that's
>probably a reasonable basic approach.
>>
>>BTW, which GPIO is the power GPIO; is it WF_EN on the schematic? That
>>seems reasonable to represent
>as a GPIO rather than a regulator since it connects directly into the
>WiFi device as a GPIO, and its use within the WiFi device can indeed be
>governed purely internally to the WiFi driver/HW. However, if this is some GPIO that controls the power to e.g.
>>VBAT3V3_IN_WF, VDDIO_WF, or other power supply to the WiFi card, then
>>it'd be better represented as a
>regulator, since the control point is outside the WiFi device.
>
>I think Rakesh may can answer this.
>
>Wei.
>---
>nvpublic


2012-06-14 04:17:31

by Wei Ni

[permalink] [raw]
Subject: RE: Where to power on the wifi device before loading the driver.

Hi, Franky

On Thu, Jun 14, 2012 at 01:33:13, Franky Lin wrote:
>Hi Wei,
>
>On 06/13/2012 03:40 AM, Wei Ni wrote:
>> In the brcmfmac init routine, it call sdio_register_driver() to
>> register driver, if the wifi device is powered on, then the mmc
>> driver will enumerate it, and call the probe callback routine.
>>
>> On the Cardhu, the wifi's power is controlled by two gpios
>> (power-gpio and reset-gpio), the default state is power-off.
>> So we need to power on it before calling sdio_register_driver(),
>> if not, the mmc driver can't enumerate it, and will not call
>> the probe routine.
>> This power on sequence is:
>> set power-gpio to 1 ;
>> mdelay(100) ;
>> set reset-gpio to 1 ;
>> mdelay(200);
>>
>> My question is where to power on the wifi. We may have three places
>> to power on it:
>> 1. power on it in the brcmfmac driver before calling
>> sdio_register_driver(). But I think this power sequence is special for
>> tegra30 cardhu, it's not good to add it in the generic wifi driver,
>> because different board may use the different way to power on the wifi.
>> 2. power on it in the mmc driver. In our tegra SD driver, it has
>> power-gpios property, which allow the slot to be powered. But this power is
>> for mmc slot, could we add this wifi power sequence in the tegra SD driver?
>> 3. hard-coded into DT. Set these gpios in the DT, something like
>> pinmux settings, but in this way, it's not good to put the mdelay() value
>> in the DT.
>
>We introduced oob interrupt support in 3.5 [1]. We are using a virtual
>platform device to retrieve board specific oob interrupt GPIO setting.
>You should be able to implement the power control in this way as well.
>Brcmfmac gets the GPIOs through platform device interface, powers up
>the chip and triggers a card detection. Then 4329 should be enumerated
>by MMC stack. The reason we didn't implement this is the card detection.
>Some design doesn't have hardware card detection since the WiFi chip is
>already on board. And the current MMC stack doesn't have virtual card
>detection interface.

Wei: Yes, in OOB mode, it's easy to add a device node in the DT, to pass
the gpio property to brcmfmac driver. But the NO-OOB mode doesn?t use the
virtual platform device, so we can't pass the gpio to driver. We need to find
a way to support these two mode both.
Could we use the virtual platform device both for OOB and NO-OOB mode? so that
we can implement power control in these two mode, and we can add a flags to
control if need to power on or not.
BTW, does that power on sequence is generally for 4329?

>
>Franky
>
>[1]: http://www.spinics.net/lists/linux-wireless/msg89335.html




2012-06-26 09:01:19

by Dong Aisheng

[permalink] [raw]
Subject: Re: Where to power on the wifi device before loading the driver.

On Tue, Jun 19, 2012 at 06:01:54PM -0600, Stephen Warren wrote:
> On 06/19/2012 03:44 AM, Wei Ni wrote:
> > On Tue, Jun 19, 2012 at 17:17:19, Mark Brown wrote:
> >> * PGP Signed by an unknown key
> >>
> >> On Tue, Jun 19, 2012 at 12:25:58PM +0800, Wei Ni wrote:
> >>
> >> As Stephen previously said please fix your mail formatting - word
> >> wrapping within paragraphs is important!
> >>
> >>> On Tue, Jun 19, 2012 at 09:23:35, Philip Rakity wrote:
> >>>> Why can't you use the regulator notify to get called back on power
> >>>> or voltage change options to plumb in the manipulation of the gpio.
> >>>> I can imagine you might need to add a notify call in core.c if you
> >>>> need to assert the gpio before power is applied
> >>>
> >>> Yes, we can use the regulator notify, but the Tegra30 support is via
> >>> device tree, I think there have no special board file to run the call
> >>> back.
> >>
> >> You can mix board files with device tree for cases where device tree is
> >> not yet up to the job, though the goal should be to come up with a way of expressing things in device tree.
> >
> > I think we wish to use the device tree to handle these gpios, pass them to the related driver to assert them.
> > If we mix the board files, we even can set these gpio directly, not use the notify. But I think this is not a goog way.
>
> Yes, I would definitely not want to add this to a board file; there is
> no board file for Cardhu, and we're getting close to deleting all the
> board files that are left for other boards.
>
> I think what we need is some way of matching a DT node to a device even
> when that device was instantiated through some probing mechanism such as
> SDIO or USB (and I've heard hints that one can already do this for PCI;
> I should investigate).
>
> So, we start off with the plain SDHCI controller node:
>
> sdhci@78000000 {
> compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
> reg = <0x78000000 0x200>;
> interrupts = <0 14 0x04>;
> };
>
> Then, we add a child node to represent the device that's attached:
>
> sdhci@78000000 {
> compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
> reg = <0x78000000 0x200>;
> interrupts = <0 14 0x04>;
>
> sdio-device {
> reset-gpios = <...>;
> enable-gpios = <...>;
> };
> };
>
> When the SDHCI controller/core instantiates the child device it finds on
> the HW bus, it initializes that struct device's of_node with a pointer
> to the sdio-device node above. From there, the child device can get DT
> properties such as GPIOs in the usual way.
>
> However, there are still some issues that need thought here; what if (as
> is I believe the case on Cardhu) we can in fact plug in multiple
> different types of device into the socket? Might we end up with
> something like:
>
> sdhci@78000000 {
> compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
> reg = <0x78000000 0x200>;
> interrupts = <0 14 0x04>;
>
> sdio-device-option@0 {
> compatible = "broadcom,bcm4239";
> reset-gpios = <...>;
> enable-gpios = <...>;
> };
>
> sdio-device-option@1 {
> compatible = "broadcom,bcm4330";
> reset-gpios = <...>;
> enable-gpios = <...>;
> };
> };
>
> and match on compatible value?
>
I like this idea.
We may extend the sdio_device_id to contain a compatible string,
then sdio core is responsible to pass the corresponding device node to the
sdio function driver once a match device found, this is sdio function independent.
The sdio function driver then are responsible to parse the device node to
fetch the needed information like reset-gpios and enable-gpios to do correct
initialization sequence.

However, one known question for this model is that some WiFi may need reset the chip
via gpio first before being able to be enumerated.
Then this model may not work for such devices since it can not enumerate this device
before completing the sdio function specific initialization, neither passing node to its
driver.
(i'm not sure this is a correct behavior since per my understanding, the sdio io function
(function 1-7) specific initialization should not affect the normal sdio common function
(function 0) enumeration, but i did not find a clear definition in sdio spec.
Anyone see such real devices?)

Then for such devices, that is definitely a problem.
If the sdio function specific initialization affects the normal enumeration,
then it definitely need to be put into sdio core as an interface like sdio_prepare before the
enumeration.
But it seems not reasonable since we did not know prepare for which sdio device
before the sdio device enumeration completes.
Maybe the possible solution is still do it in platform specific code via
of_machine_is_compatible(....).

For your question that user may plug multi types of sdio device with difference
gpio using,
did we see such a real device that the pluggable sdio devices need
some extra gpio to work? I did not meet it before.
If it real exists, can we rather consider it an integrated sdio devices on board?
Then we can only define the integrated sdio device node on board under sdio controller.

> But in this case, at least some of the data those two drivers want from
> that node is the same; the 2 GPIOs that control the reset and enable
> signals. Do we just make the bindings for those two devices happen to
> share the same properties so that we can get away with just one node
> irrespective of what device type has been found? What we're really
Maybe no.
These things are board specific and should be put in board dts file.
If really using the same gpios by chance, then add them for each board may
be reasonable.

> modeling is the card slot, and the services it provides, so perhaps we
> should really end up with:
>
> sdhci@78000000 {
> compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
> reg = <0x78000000 0x200>;
> interrupts = <0 14 0x04>;
>
> socket {
> reset-gpios = <...>;
> enable-gpios = <...>;
>
> sdio-device-option@0 {
> compatible = "broadcom,bcm4239";
> };
>
> sdio-device-option@1 {
> compatible = "broadcom,bcm4330";
> };
> };
> };
>
> Where somehow the WiFi drivers can obtain the reset/enable service from
> the socket that hosts them, but still have nodes for the attached device
> itself in case any additional driver-specific configuration is needed.
>
> That sounds somewhat similar to the virtual platform device that was
> mentioned earlier in this thread by Broadcom, although I'm not sure
> whether explicitly modeling it as a platform device makes sense, rather
> than the socket providing services to the WiFi module, not necessarily
> through a device.
>
> Unfortunately, it should be noted that the WiFi device socket on Cardhu
> apparently isn't some standardized thing (like PCIe) but something
> rather more custom, although there are apparently devices available to
> plug into it from multiple module vendors which contain WiFi chips from
> multiple chip vendors.
That seems should be cared by the corresponding WiFi function driver
rather than the binding itself no matter which is plugged in.
The binding just pass the node to driver, then everything else is driver
specific. Is that reasonable?

Regards
Dong Aisheng
>
> Anyway, these are just some quick thoughts on the topic - obviously more
> flesh is needed.
> _______________________________________________
> devicetree-discuss mailing list
> [email protected]
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>


2012-06-18 08:03:29

by Laxman Dewangan

[permalink] [raw]
Subject: RE: Where to power on the wifi device before loading the driver.

If it is for power then it has to go via regulator. It does not make sense to directly control the gpio inside the wifi driver.


-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Rakesh Kumar
Sent: Monday, June 18, 2012 1:11 PM
To: Wei Ni; 'Stephen Warren'
Cc: Mark Brown; '[email protected]'; Thierry Reding; Mursalin Akon; '[email protected]'; [email protected]; '[email protected]'; [email protected]; [email protected]
Subject: RE: Where to power on the wifi device before loading the driver.

>Now if this means adding a child node under the SDIO controller to represent the attached device, and storing any settings required by that device in that child node, that's probably a reasonable basic approach.
>
>BTW, which GPIO is the power GPIO; is it WF_EN on the schematic? That seems reasonable to represent as a GPIO rather than a regulator since it connects directly into the WiFi device as a GPIO, and its use within the WiFi device can indeed be governed purely internally to the WiFi driver/HW. However, if this is some GPIO that controls the power to e.g.
>VBAT3V3_IN_WF, VDDIO_WF, or other power supply to the WiFi card, then it'd be better represented as a regulator, since the control point is outside the WiFi device.

Tegra uses two GPIO (WF_EN and WF_RST) to power on and reset bcm4329 card. In case of bcm4329, these two lines are shorted. Tegra does not control VBAT3V3_IN_WF, VDDIO_WF, or other power supply to the WiFi card based on these GPIO. Uses of these GPIO are internal to WiFi HW. It is reasonable to represent as a GPU rather than regulator.

Rakesh
---
nvpublic

-----Original Message-----
From: Wei Ni
Sent: 18 June 2012 11:50
To: 'Stephen Warren'; Rakesh Kumar
Cc: Mark Brown; '[email protected]'; Thierry Reding; Mursalin Akon; '[email protected]'; [email protected]; '[email protected]'; [email protected]; [email protected]
Subject: RE: Where to power on the wifi device before loading the driver.

On Fri, Jun 15, 2012 at 23:49:10, Stephen Warren wrote:
>> I talked with Franky, this power sequence is generally for 4329, so
>> it mean this sequence can be put into the wifi driver.
>> We can use the virtual platform device both for OOB and non OOB.
>> I will send out patches later.
>
>Can you please expand on what a "virtual platform device" is; device tree typically represents real hardware rather than anything "virtual".

The "virtual platform device" is created before the real wifi device, so that it can pass the gpios to the driver, and power up the device before calling sdio_register_driver.
But as Franky said, this power sequence is only generally for 4329, not for the USB dongle chips. So it's not a good idea to add this "power up" in brcmfmac.

>
>Now if this means adding a child node under the SDIO controller to represent the attached device, and storing any settings required by that device in that child node, that's probably a reasonable basic approach.
>
>BTW, which GPIO is the power GPIO; is it WF_EN on the schematic? That seems reasonable to represent as a GPIO rather than a regulator since it connects directly into the WiFi device as a GPIO, and its use within the WiFi device can indeed be governed purely internally to the WiFi driver/HW. However, if this is some GPIO that controls the power to e.g.
>VBAT3V3_IN_WF, VDDIO_WF, or other power supply to the WiFi card, then it'd be better represented as a regulator, since the control point is outside the WiFi device.

I think Rakesh may can answer this.

Wei.
---
nvpublic

2012-06-19 01:27:49

by Philip Rakity

[permalink] [raw]
Subject: RE: Where to power on the wifi device before loading the driver.

Why can't you use the regulator notify to get called back on power or voltage change options to plumb in the manipulation of the gpio. I can imagine you might need to add a notify call in core.c if you need to assert the gpio before power is applied


________________________________________
From: [email protected] [[email protected]] On Behalf Of Wei Ni [[email protected]]
Sent: Sunday, June 17, 2012 11:20 PM
To: 'Stephen Warren'; Rakesh Kumar
Cc: Mark Brown; '[email protected]'; Thierry Reding; Mursalin Akon; '[email protected]'; [email protected]; '[email protected]'; [email protected]; [email protected]
Subject: RE: Where to power on the wifi device before loading the driver.

On Fri, Jun 15, 2012 at 23:49:10, Stephen Warren wrote:
>> I talked with Franky, this power sequence is generally for 4329, so
>> it mean this sequence can be put into the wifi driver.
>> We can use the virtual platform device both for OOB and non OOB.
>> I will send out patches later.
>
>Can you please expand on what a "virtual platform device" is; device tree typically represents real hardware rather than anything "virtual".

The "virtual platform device" is created before the real wifi device, so that it can pass the gpios to the driver, and power up the device before calling sdio_register_driver.
But as Franky said, this power sequence is only generally for 4329, not for the USB dongle chips. So it's not a good idea to add this "power up" in brcmfmac.

>
>Now if this means adding a child node under the SDIO controller to represent the attached device, and storing any settings required by that device in that child node, that's probably a reasonable basic approach.
>
>BTW, which GPIO is the power GPIO; is it WF_EN on the schematic? That seems reasonable to represent as a GPIO rather than a regulator since it connects directly into the WiFi device as a GPIO, and its use within the WiFi device can indeed be governed purely internally to the WiFi driver/HW. However, if this is some GPIO that controls the power to e.g.
>VBAT3V3_IN_WF, VDDIO_WF, or other power supply to the WiFi card, then it'd be better represented as a regulator, since the control point is outside the WiFi device.

I think Rakesh may can answer this.

Wei.
---
nvpublic

2012-06-18 06:20:18

by Wei Ni

[permalink] [raw]
Subject: RE: Where to power on the wifi device before loading the driver.

On Fri, Jun 15, 2012 at 23:49:10, Stephen Warren wrote:
>> I talked with Franky, this power sequence is generally for 4329, so
>> it mean this sequence can be put into the wifi driver.
>> We can use the virtual platform device both for OOB and non OOB.
>> I will send out patches later.
>
>Can you please expand on what a "virtual platform device" is; device tree typically represents real hardware rather than anything "virtual".

The "virtual platform device" is created before the real wifi device, so that it can pass the gpios to the driver, and power up the device before calling sdio_register_driver.
But as Franky said, this power sequence is only generally for 4329, not for the USB dongle chips. So it's not a good idea to add this "power up" in brcmfmac.

>
>Now if this means adding a child node under the SDIO controller to represent the attached device, and storing any settings required by that device in that child node, that's probably a reasonable basic approach.
>
>BTW, which GPIO is the power GPIO; is it WF_EN on the schematic? That seems reasonable to represent as a GPIO rather than a regulator since it connects directly into the WiFi device as a GPIO, and its use within the WiFi device can indeed be governed purely internally to the WiFi driver/HW. However, if this is some GPIO that controls the power to e.g.
>VBAT3V3_IN_WF, VDDIO_WF, or other power supply to the WiFi card, then it'd be better represented as a regulator, since the control point is outside the WiFi device.

I think Rakesh may can answer this.

Wei.
---
nvpublic



2012-06-20 11:28:56

by Wei Ni

[permalink] [raw]
Subject: RE: Where to power on the wifi device before loading the driver.

On Wed, Jun 20, 2012 at 08:01:54, Stephen Warren wrote:
>On 06/19/2012 03:44 AM, Wei Ni wrote:
>> On Tue, Jun 19, 2012 at 17:17:19, Mark Brown wrote:
>>> * PGP Signed by an unknown key
>>>
>>> On Tue, Jun 19, 2012 at 12:25:58PM +0800, Wei Ni wrote:
>>>
>>> As Stephen previously said please fix your mail formatting - word
>>> wrapping within paragraphs is important!
>>>
>>>> On Tue, Jun 19, 2012 at 09:23:35, Philip Rakity wrote:
>>>>> Why can't you use the regulator notify to get called back on power
>>>>> or voltage change options to plumb in the manipulation of the gpio.
>>>>> I can imagine you might need to add a notify call in core.c if you
>>>>> need to assert the gpio before power is applied
>>>>
>>>> Yes, we can use the regulator notify, but the Tegra30 support is
>>>> via device tree, I think there have no special board file to run
>>>> the call back.
>>>
>>> You can mix board files with device tree for cases where device tree
>>> is not yet up to the job, though the goal should be to come up with
>>> a way of expressing things in
>device tree.
>>
>> I think we wish to use the device tree to handle these gpios, pass
>> them to the related driver to
>assert them.
>> If we mix the board files, we even can set these gpio directly, not
>> use the notify. But I think this
>is not a goog way.
>
>Yes, I would definitely not want to add this to a board file; there is
>no board file for Cardhu, and we're getting close to deleting all the board files that are left for other boards.
>
>I think what we need is some way of matching a DT node to a device even
>when that device was instantiated through some probing mechanism such
>as SDIO or USB (and I've heard hints that one can already do this for PCI; I should investigate).
>
>So, we start off with the plain SDHCI controller node:
>
>sdhci@78000000 {
> compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
> reg = <0x78000000 0x200>;
> interrupts = <0 14 0x04>;
>};
>
>Then, we add a child node to represent the device that's attached:
>
>sdhci@78000000 {
> compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
> reg = <0x78000000 0x200>;
> interrupts = <0 14 0x04>;
>
> sdio-device {
> reset-gpios = <...>;
> enable-gpios = <...>;
> };
>};
>
>When the SDHCI controller/core instantiates the child device it finds
>on the HW bus, it initializes that struct device's of_node with a
>pointer to the sdio-device node above. From there, the child device can get DT properties such as GPIOs in the usual way.
>
>However, there are still some issues that need thought here; what if
>(as is I believe the case on
>Cardhu) we can in fact plug in multiple different types of device into
>the socket? Might we end up with something like:
>
>sdhci@78000000 {
> compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
> reg = <0x78000000 0x200>;
> interrupts = <0 14 0x04>;
>
> sdio-device-option@0 {
> compatible = "broadcom,bcm4239";
> reset-gpios = <...>;
> enable-gpios = <...>;
> };
>
> sdio-device-option@1 {
> compatible = "broadcom,bcm4330";
> reset-gpios = <...>;
> enable-gpios = <...>;
> };
>};
>
>and match on compatible value?
>
>But in this case, at least some of the data those two drivers want from
>that node is the same; the 2 GPIOs that control the reset and enable
>signals. Do we just make the bindings for those two devices happen to
>share the same properties so that we can get away with just one node
>irrespective of what device type has been found? What we're really modeling is the card slot, and the services it provides, so perhaps we should really end up with:
>
>sdhci@78000000 {
> compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
> reg = <0x78000000 0x200>;
> interrupts = <0 14 0x04>;
>
> socket {
> reset-gpios = <...>;
> enable-gpios = <...>;
>
> sdio-device-option@0 {
> compatible = "broadcom,bcm4239";
> };
>
> sdio-device-option@1 {
> compatible = "broadcom,bcm4330";
> };
> };
>};
>
>Where somehow the WiFi drivers can obtain the reset/enable service from
>the socket that hosts them, but still have nodes for the attached
>device itself in case any additional driver-specific configuration is needed.
>
>That sounds somewhat similar to the virtual platform device that was
>mentioned earlier in this thread by Broadcom, although I'm not sure
>whether explicitly modeling it as a platform device makes sense, rather than the socket providing services to the WiFi module, not necessarily through a device.
>
>Unfortunately, it should be noted that the WiFi device socket on Cardhu
>apparently isn't some standardized thing (like PCIe) but something
>rather more custom, although there are apparently devices available to
>plug into it from multiple module vendors which contain WiFi chips from multiple chip vendors.
>
>Anyway, these are just some quick thoughts on the topic - obviously more flesh is needed.

Stephen, as Rakesh said that in case of bcm4329, the two GPIO (WF_EN and WF_RST) are shorted. So I think we can simply to use the power-gpios properity in the SD driver, just use one gpio.
I test it in this way, it runs ok, the bcm4329 can be powered up. We can use this option.

---
nvpublic


2012-06-14 15:54:26

by Stephen Warren

[permalink] [raw]
Subject: Re: Where to power on the wifi device before loading the driver.

On 06/14/2012 06:12 AM, Mark Brown wrote:
> On Thu, Jun 14, 2012 at 08:31:20AM +0200, Thierry Reding wrote:
>> On Wed, Jun 13, 2012 at 03:17:06PM -0600, Stephen Warren wrote:
>
>>> The core of the issue is that:
>
>>> * Tegra30 support is via device tree. * We have an SDIO bus,
>>> and the WiFi device attached to that bus is enumerable. * Since
>>> the WiFi device is enumerable, no node exists in the DT to
>>> represent it. * However, the driver for the WiFi device needs
>>> certain information, such as the reset GPIO ID and perhaps
>>> power GPIO.
>
>> PCI devices are also enumerable and yet they can be matched up
>> with nodes in the device tree. Perhaps something similar could be
>> added for the SDIO bus?
>
> This seems to make the most sense - pushing this through the
> regulator API is just a bodge.

Yes, that seems reasonable.

Presumably the power GPIO should be a fixed regulator though, since it
is a power control not just a plain old GPIO? That said, the current
driver apparently deals with this as a GPIO already.

The reset GPIO can separately/directly controlled by the WiFi driver
though.

2012-06-19 09:13:52

by Wei Ni

[permalink] [raw]
Subject: RE: Where to power on the wifi device before loading the driver.

On Mon, Jun 18, 2012 at 23:01:45, Stephen Warren wrote:
>On 06/18/2012 02:03 AM, Laxman Dewangan wrote:
>> Rakesh Kumar wrote at Monday, June 18, 2012 1:11 PM:
>> > Stephen Warren wrote:
>> >> Now if this means adding a child node under the SDIO controller to
>> >> represent the attached device, and storing any settings required
>> >> by that device in that child node, that's probably a reasonable
>> >> basic approach.
>> >>
>> >> BTW, which GPIO is the power GPIO; is it WF_EN on the schematic?
>> >> That seems reasonable to represent as a GPIO rather than a
>> >> regulator since it connects directly into the WiFi device as a
>> >> GPIO, and its use within the WiFi device can indeed be governed
>> >> purely internally to the WiFi driver/HW. However, if this is some
>> >> GPIO that controls the power to e.g. VBAT3V3_IN_WF, VDDIO_WF, or
>> >> other power supply to the WiFi card, then it'd be better
>> >> represented as a regulator, since the control point is outside the
>> >> WiFi device.
>> >
>> > Tegra uses two GPIO (WF_EN and WF_RST) to power on and reset
>> > bcm4329 card. In case of bcm4329, these two lines are shorted.
>> > Tegra does not control VBAT3V3_IN_WF, VDDIO_WF, or other power
>> > supply to the WiFi card based on these GPIO. Uses of these GPIO are
>> > internal to WiFi HW. It is reasonable to represent as a GPU rather
>> > than regulator.
>>
>> If it is for power then it has to go via regulator. It does not make
>> sense to directly control the gpio inside the wifi driver.
>
>As far as the board goes, WF_EN is just a GPIO signal to the WiFi card;
>it doesn't gate power to the card. If it gates power inside the card,
>that's an internal implementation detail of the card, and something I'd
>be fine with the driver knowing directly, and hence I'm OK with representing
>this as a GPIO rather than a regulator - that's what it is externally to the WiFi device.

Because these gpio is just for wifi device, could we use the rfkill-gpio driver for this isse? We just need to add DT support in rfkill-gpio driver and add a device node in the DT. And the user also can use the rfkill interface to control the wifi device.

>
>(BTW everyone, many of the emails in this thread are awfully formatted
>- top-posted and not word- wrapped. It's very hard to read them... I tried to reformat everything and fix it in this email)



2012-06-14 11:44:19

by Wei Ni

[permalink] [raw]
Subject: RE: Where to power on the wifi device before loading the driver.

On Thu, Jun 14, 2012 at 19:27:50, Wei Ni wrote:
>On Thu, Jun 14, 2012 at 05:17:06, Stephen Warren wrote:
>>On 06/13/2012 04:40 AM, Wei Ni wrote:
>>> Hi, all
>>> I'm working on the tegra30 wifi upstream issue.
>>>
>>> The tegra30 board (Cardhu) use Broadcom 4329 as wifi device, and use brcmfmac as the wifi driver.
>>>
>>> In the brcmfmac init routine, it call sdio_register_driver() to
>>> register driver, if the wifi device
>is powered on, then the mmc driver will enumerate it, and call the probe callback routine.
>>>
>>> On the Cardhu, the wifi's power is controlled by two gpios
>>> (power-gpio and reset-gpio), the default state is power-off. So we
>>> need to power on it before
>calling sdio_register_driver(), if not, the mmc driver can't enumerate
>it, and will not call the probe routine.
>>> This power on sequence is:
>>> set power-gpio to 1 ;
>>> mdelay(100) ;
>>> set reset-gpio to 1 ;
>>> mdelay(200);
>>>
>>> My question is where to power on the wifi. We may have three places to power on it:
>>> 1. power on it in the brcmfmac driver before calling
>>> sdio_register_driver(). But I think this power sequence is special
>>> for tegra30 cardhu, it's not
>good to add it in the generic wifi driver, because different board may
>use the different way to power on the wifi.
>>> 2. power on it in the mmc driver. In our tegra SD driver, it has
>>> power-gpios property, which allow the slot to be powered. But this
>>> power is for mmc slot, could we
>add this wifi power sequence in the tegra SD driver?
>>> 3. hard-coded into DT. Set these gpios in the DT, something like
>>> pinmux settings, but in this way,
>it's not good to put the mdelay() value in the DT.
>>>
>>> I have no good idea for it, does anyone has suggestion?
>>
>>The core of the issue is that:
>>
>>* Tegra30 support is via device tree.
>>* We have an SDIO bus, and the WiFi device attached to that bus is enumerable.
>>* Since the WiFi device is enumerable, no node exists in the DT to represent it.
>>* However, the driver for the WiFi device needs certain information,
>>such as the reset GPIO ID and perhaps power GPIO.
>>
>>For the power GPIO, it seems reasonable to either use the existing
>>Tegra SD controller's power-gpios DT property, or replace that
>>property with a real regulator binding. The SD driver would then control this.
>>Still, that approach would mean the WiFi driver wouldn't be able to
>>control power to the device
>directly, which might not be a good thing.
>>
>>However, I'm not sure that the reset GPIO is also something that
>>should be controlled by the SD card driver; it seems like it's much
>>more closely related to the WiFi
>device/driver.
>>
>>I wonder if the power and reset GPIO shouldn't be represented as a
>>combined custom regulator type, which knows how to sequence multiple GPIOs.
>>
>>Perhaps SDIO "client" devices also need a way to communicate with
>>their "host port" to obtain services such as power and reset control?
>>
>>This is all very similar to the WiFi rfkill discussion we have re: the
>>Toshiba AC100 a little while back, although that was USB rather than SDIO.
>
>In the rfkill-gpio, it already have the reset-gpio and shutdown-gpio.
>I test it, add a device node in the DT to pass the gpio properties.
>It will use these gpios to power up/off , but its default power sequence is different with the 4329.
>It block==0, it mean power up, it will set reset-gpio first, then set power-gpio next.
>I think it should set power-gpio first, is it right?
>
I test with the rfkill-gpio's default power sequence just now, it also can power up the device.
It seems we can use this rfkill-gpio driver.

---
nvpublic


2012-06-18 07:40:51

by Rakesh Kumar

[permalink] [raw]
Subject: RE: Where to power on the wifi device before loading the driver.

>Now if this means adding a child node under the SDIO controller to represent the attached device, and storing any settings required by that device in that child node, that's probably a reasonable basic approach.
>
>BTW, which GPIO is the power GPIO; is it WF_EN on the schematic? That seems reasonable to represent as a GPIO rather than a regulator since it connects directly into the WiFi device as a GPIO, and its use within the WiFi device can indeed be governed purely internally to the WiFi driver/HW. However, if this is some GPIO that controls the power to e.g.
>VBAT3V3_IN_WF, VDDIO_WF, or other power supply to the WiFi card, then it'd be better represented as a regulator, since the control point is outside the WiFi device.

Tegra uses two GPIO (WF_EN and WF_RST) to power on and reset bcm4329 card. In case of bcm4329, these two lines are shorted. Tegra does not control VBAT3V3_IN_WF, VDDIO_WF, or other power supply to the WiFi card based on these GPIO. Uses of these GPIO are internal to WiFi HW. It is reasonable to represent as a GPU rather than regulator.

Rakesh
---
nvpublic

-----Original Message-----
From: Wei Ni
Sent: 18 June 2012 11:50
To: 'Stephen Warren'; Rakesh Kumar
Cc: Mark Brown; '[email protected]'; Thierry Reding; Mursalin Akon; '[email protected]'; [email protected]; '[email protected]'; [email protected]; [email protected]
Subject: RE: Where to power on the wifi device before loading the driver.

On Fri, Jun 15, 2012 at 23:49:10, Stephen Warren wrote:
>> I talked with Franky, this power sequence is generally for 4329, so
>> it mean this sequence can be put into the wifi driver.
>> We can use the virtual platform device both for OOB and non OOB.
>> I will send out patches later.
>
>Can you please expand on what a "virtual platform device" is; device tree typically represents real hardware rather than anything "virtual".

The "virtual platform device" is created before the real wifi device, so that it can pass the gpios to the driver, and power up the device before calling sdio_register_driver.
But as Franky said, this power sequence is only generally for 4329, not for the USB dongle chips. So it's not a good idea to add this "power up" in brcmfmac.

>
>Now if this means adding a child node under the SDIO controller to represent the attached device, and storing any settings required by that device in that child node, that's probably a reasonable basic approach.
>
>BTW, which GPIO is the power GPIO; is it WF_EN on the schematic? That seems reasonable to represent as a GPIO rather than a regulator since it connects directly into the WiFi device as a GPIO, and its use within the WiFi device can indeed be governed purely internally to the WiFi driver/HW. However, if this is some GPIO that controls the power to e.g.
>VBAT3V3_IN_WF, VDDIO_WF, or other power supply to the WiFi card, then it'd be better represented as a regulator, since the control point is outside the WiFi device.

I think Rakesh may can answer this.

Wei.
---
nvpublic



2012-06-14 12:12:37

by Mark Brown

[permalink] [raw]
Subject: Re: Where to power on the wifi device before loading the driver.

On Thu, Jun 14, 2012 at 08:31:20AM +0200, Thierry Reding wrote:
> On Wed, Jun 13, 2012 at 03:17:06PM -0600, Stephen Warren wrote:

> > The core of the issue is that:

> > * Tegra30 support is via device tree.
> > * We have an SDIO bus, and the WiFi device attached to that bus is
> > enumerable.
> > * Since the WiFi device is enumerable, no node exists in the DT to
> > represent it.
> > * However, the driver for the WiFi device needs certain information,
> > such as the reset GPIO ID and perhaps power GPIO.

> PCI devices are also enumerable and yet they can be matched up with nodes in
> the device tree. Perhaps something similar could be added for the SDIO bus?

This seems to make the most sense - pushing this through the regulator
API is just a bodge.


Attachments:
(No filename) (763.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-06-19 09:44:09

by Wei Ni

[permalink] [raw]
Subject: RE: Where to power on the wifi device before loading the driver.

On Tue, Jun 19, 2012 at 17:17:19, Mark Brown wrote:
>* PGP Signed by an unknown key
>
>On Tue, Jun 19, 2012 at 12:25:58PM +0800, Wei Ni wrote:
>
>As Stephen previously said please fix your mail formatting - word
>wrapping within paragraphs is important!
>
>> On Tue, Jun 19, 2012 at 09:23:35, Philip Rakity wrote:
>> >Why can't you use the regulator notify to get called back on power
>> >or voltage change options to plumb in the manipulation of the gpio.
>> >I can imagine you might need to add a notify call in core.c if you
>> >need to assert the gpio before power is applied
>>
>> Yes, we can use the regulator notify, but the Tegra30 support is via
>> device tree, I think there have no special board file to run the call
>> back.
>
>You can mix board files with device tree for cases where device tree is
>not yet up to the job, though the goal should be to come up with a way of expressing things in device tree.

I think we wish to use the device tree to handle these gpios, pass them to the related driver to assert them.
If we mix the board files, we even can set these gpio directly, not use the notify. But I think this is not a goog way.

Wei.
---
nvpublic

>
>* Unknown Key
>* 0x6E30FDDD



2012-06-18 06:00:38

by Wei Ni

[permalink] [raw]
Subject: RE: Where to power on the wifi device before loading the driver.

On Sat, Jun 16, 2012 at 00:24:41, Franky Lin wrote:
>On 06/14/2012 11:09 PM, Wei Ni wrote:
>> I talked with Franky, this power sequence is generally for 4329, so it mean this sequence can be put into the wifi driver.
>
>Hold on. It's generally for 4329. But fullmac supports 4329/4330 along
>with USB dongle chips. Also more chips are incoming. I dont think it's a good idea to add *this* power sequence into brcmfmac.

Ok we need find other ways for it. Thanks for your comments.

Wei
---
nvpublic



2012-06-14 16:46:55

by Franky Lin

[permalink] [raw]
Subject: Re: Where to power on the wifi device before loading the driver.

On 06/13/2012 09:17 PM, Wei Ni wrote:
> Wei: Yes, in OOB mode, it's easy to add a device node in the DT, to pass
> the gpio property to brcmfmac driver. But the NO-OOB mode doesn?t use the
> virtual platform device, so we can't pass the gpio to driver. We need to find
> a way to support these two mode both.
> Could we use the virtual platform device both for OOB and NO-OOB mode? so that
> we can implement power control in these two mode, and we can add a flags to
> control if need to power on or not.

Of course you can use it for non OOB.

> BTW, does that power on sequence is generally for 4329?

Yes.

Regards.
Franky


2012-06-13 21:17:11

by Stephen Warren

[permalink] [raw]
Subject: Re: Where to power on the wifi device before loading the driver.

On 06/13/2012 04:40 AM, Wei Ni wrote:
> Hi, all
> I'm working on the tegra30 wifi upstream issue.
>
> The tegra30 board (Cardhu) use Broadcom 4329 as wifi device, and use brcmfmac as the wifi driver.
>
> In the brcmfmac init routine, it call sdio_register_driver() to register driver, if the wifi device is powered on, then the mmc driver will enumerate it, and call the probe callback routine.
>
> On the Cardhu, the wifi's power is controlled by two gpios (power-gpio and reset-gpio), the default state is power-off. So we need to power on it before calling sdio_register_driver(), if not, the mmc driver can't enumerate it, and will not call the probe routine.
> This power on sequence is:
> set power-gpio to 1 ;
> mdelay(100) ;
> set reset-gpio to 1 ;
> mdelay(200);
>
> My question is where to power on the wifi. We may have three places to power on it:
> 1. power on it in the brcmfmac driver before calling sdio_register_driver(). But I think this power sequence is special for tegra30 cardhu, it's not good to add it in the generic wifi driver, because different board may use the different way to power on the wifi.
> 2. power on it in the mmc driver. In our tegra SD driver, it has power-gpios property, which allow the slot to be powered. But this power is for mmc slot, could we add this wifi power sequence in the tegra SD driver?
> 3. hard-coded into DT. Set these gpios in the DT, something like pinmux settings, but in this way, it's not good to put the mdelay() value in the DT.
>
> I have no good idea for it, does anyone has suggestion?

The core of the issue is that:

* Tegra30 support is via device tree.
* We have an SDIO bus, and the WiFi device attached to that bus is
enumerable.
* Since the WiFi device is enumerable, no node exists in the DT to
represent it.
* However, the driver for the WiFi device needs certain information,
such as the reset GPIO ID and perhaps power GPIO.

For the power GPIO, it seems reasonable to either use the existing Tegra
SD controller's power-gpios DT property, or replace that property with a
real regulator binding. The SD driver would then control this. Still,
that approach would mean the WiFi driver wouldn't be able to control
power to the device directly, which might not be a good thing.

However, I'm not sure that the reset GPIO is also something that should
be controlled by the SD card driver; it seems like it's much more
closely related to the WiFi device/driver.

I wonder if the power and reset GPIO shouldn't be represented as a
combined custom regulator type, which knows how to sequence multiple GPIOs.

Perhaps SDIO "client" devices also need a way to communicate with their
"host port" to obtain services such as power and reset control?

This is all very similar to the WiFi rfkill discussion we have re: the
Toshiba AC100 a little while back, although that was USB rather than SDIO.

2012-06-15 16:40:11

by Franky Lin

[permalink] [raw]
Subject: Re: Where to power on the wifi device before loading the driver.

On 06/14/2012 11:09 PM, Wei Ni wrote:
> I talked with Franky, this power sequence is generally for 4329, so it mean this sequence can be put into the wifi driver.

Hold on. It's generally for 4329. But fullmac supports 4329/4330 along
with USB dongle chips. Also more chips are incoming. I dont think it's a
good idea to add *this* power sequence into brcmfmac.

Franky