from:"Arend van Spriel"

Re: [PATCH v2] mmc: sunxi: Don't start commands while the card is busy

2015-07-28 Thread Arend van Spriel


On 07/21/2015 02:15 PM, Ulf Hansson wrote:

On 10 July 2015 at 17:14, Hans de Goede hdego...@redhat.com wrote:

Some sdio wifi modules have not been working reliable with the sunxi-mmc
host code. This turns out to be caused by it starting new commands while
the card signals that it is still busy processing a previous command.


Which are those commands? Or more interesting, which response types do
these commands expect?
I don't think this is a sunxi specific issue, I have seen similar
issues for other host controllers.


Indeed. A similar fix was needed for dw_mmc host controller.


I am thinking that perhaps this should be managed by the mmc core
instead of local hacks to sunxi. Potentially we could make the core to
invoke the already existing host_ops-card_busy() callback when
needed...


The problem here is that there are a number of host controllers out 
there not implementing that callback. That may be because the hardware 
is dealing properly with busy signalling, but I would not bet on that.


Regards,
Arend


Within this context, could I ask whether you controller supports IRQ
based HW-busy detection? Or you need polling of the status register?



This commit fixes this, thereby fixing the wifi reliability issues on
the Cubietruck and other sunxi boards using sdio wifi.

Reported-by: Eugene K sigintmai...@gmail.com
Suggested-by: Eugene K sigintmai...@gmail.com
Cc: Eugene K sigintmai...@gmail.com
Cc: Arend van Spriel ar...@broadcom.com
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v2:
-Properly accredit Eugene K for coming up with the fix for this
---
  drivers/mmc/host/sunxi-mmc.c | 32 
  1 file changed, 32 insertions(+)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 4d3e1ff..daa90b7 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -289,6 +289,24 @@ static int sunxi_mmc_init_host(struct mmc_host *mmc)
 return 0;
  }

+/* Wait for card to report ready before starting a new cmd */
+static int sunxi_mmc_wait_card_ready(struct sunxi_mmc_host *host)
+{
+   unsigned long expire = jiffies + msecs_to_jiffies(500);
+   u32 rval;
+
+   do {
+   rval = mmc_readl(host, REG_STAS);
+   } while (time_before(jiffies, expire)  (rval  SDXC_CARD_DATA_BUSY));
+
+   if (rval  SDXC_CARD_DATA_BUSY) {
+   dev_err(mmc_dev(host-mmc), Error R1 ready timeout\n);
+   return -EIO;
+   }
+
+   return 0;
+}
+
  static void sunxi_mmc_init_idma_des(struct sunxi_mmc_host *host,
 struct mmc_data *data)
  {
@@ -383,6 +401,8 @@ static void sunxi_mmc_send_manual_stop(struct 
sunxi_mmc_host *host,
 u32 arg, cmd_val, ri;
 unsigned long expire = jiffies + msecs_to_jiffies(1000);

+   sunxi_mmc_wait_card_ready(host);
+
 cmd_val = SDXC_START | SDXC_RESP_EXPIRE |
   SDXC_STOP_ABORT_CMD | SDXC_CHECK_RESPONSE_CRC;

@@ -597,6 +617,11 @@ static int sunxi_mmc_oclk_onoff(struct sunxi_mmc_host 
*host, u32 oclk_en)
  {
 unsigned long expire = jiffies + msecs_to_jiffies(250);
 u32 rval;
+   int ret;
+
+   ret = sunxi_mmc_wait_card_ready(host);
+   if (ret)
+   return ret;

 rval = mmc_readl(host, REG_CLKCR);
 rval = ~(SDXC_CARD_CLOCK_ON | SDXC_LOW_POWER_ON);
@@ -785,6 +810,13 @@ static void sunxi_mmc_request(struct mmc_host *mmc, struct 
mmc_request *mrq)
 return;
 }

+   ret = sunxi_mmc_wait_card_ready(host);
+   if (ret) {
+   mrq-cmd-error = ret;
+   mmc_request_done(mmc, mrq);
+   return;
+   }
+
 if (data) {
 ret = sunxi_mmc_map_dma(host, data);
 if (ret  0) {
--
2.4.3



Kind regards
Uffe



--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings

2015-06-29 Thread Arend van Spriel


On 06/19/15 21:20, Arend van Spriel wrote:

On 06/19/15 20:49, Rob Herring wrote:

On Fri, Jun 19, 2015 at 12:06 PM, Arend van Sprielar...@broadcom.com
wrote:

On 06/19/15 17:47, Rob Herring wrote:


On Thu, Jun 18, 2015 at 3:37 PM, Ilya Faensonifaen...@broadcom.com
wrote:


Hi Rob.

-Original Message-
From: linux-bluetooth-ow...@vger.kernel.org
[mailto:linux-bluetooth-ow...@vger.kernel.org] On Behalf Of Rob
Herring
Sent: Thursday, June 18, 2015 3:41 PM
To: Ilya Faenson
Cc: mar...@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org;
linux-blueto...@vger.kernel.org
Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree
bindings

On Thu, Jun 18, 2015 at 1:54 PM, Ilya Faensonifaen...@broadcom.com
wrote:


Hi Rob,



Your emails are base64 encoded. They should be plain text for the
list.

IF: The Outlook is configured to respond in the sender's format. I
therefore respond in the encoding you've used.



I assure you that that is not the case or I would be banished from
lists by now. Outlook is generally incapable of correctly sending
mails to lists. The reply header above is one aspect of that. The
other problem is your replies don't get wrapped and prefixed properly.
That could be my client I guess, but *all* other mails are fine.

My sent mails have:

Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable



-Original Message-
From: Rob Herring [mailto:robherri...@gmail.com]
Sent: Thursday, June 18, 2015 11:03 AM
To: Ilya Faenson
Cc: mar...@holtmann.org; Arend Van Spriel;
devicetree@vger.kernel.org;
linux-blueto...@vger.kernel.org
Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree
bindings

On Wed, Jun 17, 2015 at 6:11 PM, Ilya Faensonifaen...@broadcom.com
wrote:


+ devicetree lists



[...]


diff --git
a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
new file mode 100644
index 000..5dbcd57
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
@@ -0,0 +1,86 @@
+btbcm
+--
+
+Required properties:
+
+ - compatible : must be brcm,brcm-bt-uart.



You need to describe the chip, not the interface.

IF: This driver is for all Broadcom Bluetooth UART based chips.



BT only chips? Most/many Broadcom chips are combo chips. I understand
the driver for BT is *mostly* separate from other chip functions like
WiFi, GPS and NFC, but the h/w is a single chip. I say most because
likely there are some parts shared: a voltage rail, reset line, or
power down line. I think some can do BT over the SDIO interface too,
so the interface may be shared. The DT is a description of the h/w
(i.e. what part # for a chip) and not a driver data structure. You
need to describe the whole chip in the binding. It is a Linux problem
if there needs to be multiple separate drivers.

IF: Defining complete DT description for the Broadcom combo chips for
multiple interfaces is well beyond the scope of what I am doing. I
just need
to define a DT node for the input and output GPIOs connected to the
BT UART
chip. BT may or may not be part of the combo chip: it does not
really matter
for this exercise. I thought I would take this opportunity to place
some BT
device parameters into that node as well. If you're not comfortable
with
this, I could just call it a GPIO set to avoid mentioning BT and
UART at
all but it would make little sense. Still, I could consider it.
Would you
have GPIO set recommendations? Alternatively, NFC Marvell code
you're
referring to has parameters configured as Linux module parameters.
I could
do the same too, avoiding this device tree discussion. Let me know.



I don't completely follow what you mean by GPIO set, but I'm
guessing that is not the right path.


Generally speaking (pontification hat on :-)), what you're trying
to do
(description of the whole chip) is well beyond what say ACPI has
done (I was
involved some in the BT ACPI exercise a few years ago). BT UART
interface is
described in ACPI independently of other parts of the same combo chip.
Requirements like that slow down the DT development in my opinion as
companies are understandably reluctant to work with unrealistic
goals. End
of pontification. :-)



ACPI and DT are very different models in terms of abstraction and
governance. I'm not going to hash through all the details.

I'm not necessarily saying we have to have a single node, but that is
my default position. You have convince me that the functions are
completely independent and I cannot judge that if you are completely
ignoring the WiFi part. From Broadcom chips I've worked with, the
supplies at least are shared (something ACPI does not expose). Perhaps
we could fudge that and just require the same supply has to be
connected to each half. I still worry there could be other internal
inter-dependencies. Perhaps BT requires the SDIO clock to be active or
something like that. Maybe not on Broadcom chips, but on other vendors

Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings

2015-06-19 Thread Arend van Spriel


On 06/19/15 17:47, Rob Herring wrote:

On Thu, Jun 18, 2015 at 3:37 PM, Ilya Faensonifaen...@broadcom.com  wrote:

Hi Rob.

-Original Message-
From: linux-bluetooth-ow...@vger.kernel.org 
[mailto:linux-bluetooth-ow...@vger.kernel.org] On Behalf Of Rob Herring
Sent: Thursday, June 18, 2015 3:41 PM
To: Ilya Faenson
Cc: mar...@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; 
linux-blueto...@vger.kernel.org
Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings

On Thu, Jun 18, 2015 at 1:54 PM, Ilya Faensonifaen...@broadcom.com  wrote:

Hi Rob,


Your emails are base64 encoded. They should be plain text for the list.

IF: The Outlook is configured to respond in the sender's format. I therefore 
respond in the encoding you've used.


I assure you that that is not the case or I would be banished from
lists by now. Outlook is generally incapable of correctly sending
mails to lists. The reply header above is one aspect of that. The
other problem is your replies don't get wrapped and prefixed properly.
That could be my client I guess, but *all* other mails are fine.

My sent mails have:

Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable



-Original Message-
From: Rob Herring [mailto:robherri...@gmail.com]
Sent: Thursday, June 18, 2015 11:03 AM
To: Ilya Faenson
Cc: mar...@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org; 
linux-blueto...@vger.kernel.org
Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings

On Wed, Jun 17, 2015 at 6:11 PM, Ilya Faensonifaen...@broadcom.com  wrote:

+ devicetree lists


[...]


diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt 
b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
new file mode 100644
index 000..5dbcd57
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
@@ -0,0 +1,86 @@
+btbcm
+--
+
+Required properties:
+
+   - compatible : must be brcm,brcm-bt-uart.


You need to describe the chip, not the interface.

IF: This driver is for all Broadcom Bluetooth UART based chips.


BT only chips? Most/many Broadcom chips are combo chips. I understand
the driver for BT is *mostly* separate from other chip functions like
WiFi, GPS and NFC, but the h/w is a single chip. I say most because
likely there are some parts shared: a voltage rail, reset line, or
power down line. I think some can do BT over the SDIO interface too,
so the interface may be shared. The DT is a description of the h/w
(i.e. what part # for a chip) and not a driver data structure. You
need to describe the whole chip in the binding. It is a Linux problem
if there needs to be multiple separate drivers.

IF: Defining complete DT description for the Broadcom combo chips for multiple interfaces is well 
beyond the scope of what I am doing. I just need to define a DT node for the input and output GPIOs 
connected to the BT UART chip. BT may or may not be part of the combo chip: it does not really 
matter for this exercise. I thought I would take this opportunity to place some BT device 
parameters into that node as well. If you're not comfortable with this, I could just call it a 
GPIO set to avoid mentioning BT and UART at all but it would make little sense. Still, 
I could consider it. Would you have GPIO set recommendations? Alternatively, NFC 
Marvell code you're referring to has parameters configured as Linux module parameters. I could do 
the same too, avoiding this device tree discussion. Let me know.



I don't completely follow what you mean by GPIO set, but I'm
guessing that is not the right path.


Generally speaking (pontification hat on :-)), what you're trying to do 
(description of the whole chip) is well beyond what say ACPI has done (I was 
involved some in the BT ACPI exercise a few years ago). BT UART interface is 
described in ACPI independently of other parts of the same combo chip. 
Requirements like that slow down the DT development in my opinion as companies 
are understandably reluctant to work with unrealistic goals. End of 
pontification. :-)



ACPI and DT are very different models in terms of abstraction and
governance. I'm not going to hash through all the details.

I'm not necessarily saying we have to have a single node, but that is
my default position. You have convince me that the functions are
completely independent and I cannot judge that if you are completely
ignoring the WiFi part. From Broadcom chips I've worked with, the
supplies at least are shared (something ACPI does not expose). Perhaps
we could fudge that and just require the same supply has to be
connected to each half. I still worry there could be other internal
inter-dependencies. Perhaps BT requires the SDIO clock to be active or
something like that. Maybe not on Broadcom chips, but on other vendors
and I have to care about them all.


All Broadcom combo chips that I know of have separate supplies, ie. 
wl-reg-on and bt-reg

Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree bindings

2015-06-19 Thread Arend van Spriel


On 06/19/15 20:49, Rob Herring wrote:

On Fri, Jun 19, 2015 at 12:06 PM, Arend van Sprielar...@broadcom.com  wrote:

On 06/19/15 17:47, Rob Herring wrote:


On Thu, Jun 18, 2015 at 3:37 PM, Ilya Faensonifaen...@broadcom.com
wrote:


Hi Rob.

-Original Message-
From: linux-bluetooth-ow...@vger.kernel.org
[mailto:linux-bluetooth-ow...@vger.kernel.org] On Behalf Of Rob Herring
Sent: Thursday, June 18, 2015 3:41 PM
To: Ilya Faenson
Cc: mar...@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org;
linux-blueto...@vger.kernel.org
Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree
bindings

On Thu, Jun 18, 2015 at 1:54 PM, Ilya Faensonifaen...@broadcom.com
wrote:


Hi Rob,



Your emails are base64 encoded. They should be plain text for the list.

IF: The Outlook is configured to respond in the sender's format. I
therefore respond in the encoding you've used.



I assure you that that is not the case or I would be banished from
lists by now. Outlook is generally incapable of correctly sending
mails to lists. The reply header above is one aspect of that. The
other problem is your replies don't get wrapped and prefixed properly.
That could be my client I guess, but *all* other mails are fine.

My sent mails have:

Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable



-Original Message-
From: Rob Herring [mailto:robherri...@gmail.com]
Sent: Thursday, June 18, 2015 11:03 AM
To: Ilya Faenson
Cc: mar...@holtmann.org; Arend Van Spriel; devicetree@vger.kernel.org;
linux-blueto...@vger.kernel.org
Subject: Re: FW: [PATCH v4 1/4] Broadcom Bluetooth UART Device Tree
bindings

On Wed, Jun 17, 2015 at 6:11 PM, Ilya Faensonifaen...@broadcom.com
wrote:


+ devicetree lists



[...]


diff --git a/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
new file mode 100644
index 000..5dbcd57
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/bluetooth/btbcm.txt
@@ -0,0 +1,86 @@
+btbcm
+--
+
+Required properties:
+
+   - compatible : must be brcm,brcm-bt-uart.



You need to describe the chip, not the interface.

IF: This driver is for all Broadcom Bluetooth UART based chips.



BT only chips? Most/many Broadcom chips are combo chips. I understand
the driver for BT is *mostly* separate from other chip functions like
WiFi, GPS and NFC, but the h/w is a single chip. I say most because
likely there are some parts shared: a voltage rail, reset line, or
power down line. I think some can do BT over the SDIO interface too,
so the interface may be shared. The DT is a description of the h/w
(i.e. what part # for a chip) and not a driver data structure. You
need to describe the whole chip in the binding. It is a Linux problem
if there needs to be multiple separate drivers.

IF: Defining complete DT description for the Broadcom combo chips for
multiple interfaces is well beyond the scope of what I am doing. I just need
to define a DT node for the input and output GPIOs connected to the BT UART
chip. BT may or may not be part of the combo chip: it does not really matter
for this exercise. I thought I would take this opportunity to place some BT
device parameters into that node as well. If you're not comfortable with
this, I could just call it a GPIO set to avoid mentioning BT and UART at
all but it would make little sense. Still, I could consider it. Would you
have GPIO set recommendations? Alternatively, NFC Marvell code you're
referring to has parameters configured as Linux module parameters. I could
do the same too, avoiding this device tree discussion. Let me know.



I don't completely follow what you mean by GPIO set, but I'm
guessing that is not the right path.


Generally speaking (pontification hat on :-)), what you're trying to do
(description of the whole chip) is well beyond what say ACPI has done (I was
involved some in the BT ACPI exercise a few years ago). BT UART interface is
described in ACPI independently of other parts of the same combo chip.
Requirements like that slow down the DT development in my opinion as
companies are understandably reluctant to work with unrealistic goals. End
of pontification. :-)



ACPI and DT are very different models in terms of abstraction and
governance. I'm not going to hash through all the details.

I'm not necessarily saying we have to have a single node, but that is
my default position. You have convince me that the functions are
completely independent and I cannot judge that if you are completely
ignoring the WiFi part. From Broadcom chips I've worked with, the
supplies at least are shared (something ACPI does not expose). Perhaps
we could fudge that and just require the same supply has to be
connected to each half. I still worry there could be other internal
inter-dependencies. Perhaps BT requires the SDIO clock to be active or
something like that. Maybe not on Broadcom chips, but on other vendors
and I have to care about them all

Re: mmc: Driver Strength Device Property

2015-02-16 Thread Arend van Spriel

On 02/16/15 15:25, Adrian Hunter wrote:

I am in the process of adding an ACPI Device Property to specify
the driver strength (aka drive strength, driver type) for use
with eMMC/SD/SDIO cards, however the ACPI Specification Workgroup
requires that Device Properties be sufficiently generic. This
raises several questions as to what is sufficiently generic. That
is covered in a Questions section below and is followed by a draft
ACPI Device Property Definition. Any comments would be greatly
appreciated.

First some background:

What are ACPI Device Properties and how do they relate to Device Tree
-

ACPI Device Properties are key / value pairs that can be recorded
in the ACPI DSDT using a _DSD object with a specific UUID. Refer:
The ACPI specification and

http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
http://www.uefi.org/sites/default/files/resources/web-page-v2.pdf

Linux provides a common API to read ACPI Device Properties and
Device Tree properties. Refer:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/base/property.c
http://marc.info/?l=linux-kernelm=141354745011569w=4

Obviously, the common API only works when the same property name
is not defined differently for the same device or class
of devices.

What is Driver Strength
---

Both the JEDEC eMMC Specification and the SD Association Physical
Layer Specification define Driver Strength. The specifications
also use the terms Drive Strength and Driver Type for the same
thing. While the specifications deal with cards, the Host
Controller can also have a Driver Strength value, for example as
specified in the SD Host Controller Specification.

For eMMC, Driver Strength is an optional setting for the HS200 and
HS400 transfer modes.

Currently JEDEC defines:

Value Nominal Impedance Approx. capability
relative to type 0

0 50 ohm x1
1 33 ohm x1.5
2 66 ohm x0.75
3 100 ohm x0.5
4 40 ohm x1.2

For SD/SDIO, Driver Strength is an optional setting for the
UHS-I transfer modes.

The SD Association defines:

Driver Value Nominal Impedance Approx. capability
Typerelative to type 0

A 1 33 ohm x1.5
B 0 50 ohm x1
C 2 66 ohm x0.75
D 3 100 ohm x0.5

So the values are the same with the exception that eMMCs have the
additional value 4 (40 ohm). It is assumed that the values will
anyway not conflict because eMMC is not removable.

The specifications state that support for Driver Strength 0
(Driver Type B) is both mandatory and the default value.
In addition, cards supply a mask of supported Driver Strength
values. Therefore the Driver Strength value is validated against
the supported values.

Questions
-

Question 1. Should there be separate Driver Strength values for
cards and host controllers?

There is no indication from the specifications that cards and
host controllers need have the same value for Driver Strength.
That suggests that separate properties for the card and host
controller should be provided for.

Originally, I was just proposing driver-strength for the
Driver Strength of the card, but now will separate this into
card-driver-strength and host-driver-strength.

Hi Adrian,

I am not sure if it makes sense to have a 'card-driver-strength'
specification for the host controller. I have been under the impression
that the proper value for this is strongly depending on the card used in
the slot. For this reason and the fact that it is programmed in our
device we added brcm,drive-strength property in our device-tree bindings
[1].

Regards,
Arend

[1]
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt

Question 2. To what devices should the Driver Strength properties
bind?

A Driver Strength property for the host controller obviously
binds to the host controller device.

Additionally it can be assumed that Driver Strength is a property
of the board (e.g. a stronger Driver Strength needed because the
eMMC is located further from the host controller), consequently
even the Driver Strength property for the card should bind against
the host controller.

Question 3. Should there be separate Driver Strength values for
different transfer modes?

Different transfer modes can have different timing requirements
which implies that there can be different Driver Strength values
for each transfer mode. To support that, there

Re: mmc: Driver Strength Device Property

2015-02-16 Thread Arend van Spriel

On 02/16/15 22:47, Adrian Hunter wrote:

On 16/02/2015 7:47 p.m., Philip Rakity wrote:

On Feb 16, 2015, at 5:39 PM, Arend van Spriel ar...@broadcom.com wrote:

On 02/16/15 15:25, Adrian Hunter wrote:

First some background:

What are ACPI Device Properties and how do they relate to Device Tree
-

ACPI Device Properties are key / value pairs that can be recorded
in the ACPI DSDT using a _DSD object with a specific UUID. Refer:
The ACPI specification and
http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf

http://www.uefi.org/sites/default/files/resources/web-page-v2.pdf

Linux provides a common API to read ACPI Device Properties and
Device Tree properties. Refer:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/base/property.c

http://marc.info/?l=linux-kernelm=141354745011569w=4

Obviously, the common API only works when the same property name
is not defined differently for the same device or class
of devices.

What is Driver Strength
---

For eMMC, Driver Strength is an optional setting for the HS200 and
HS400 transfer modes.

Currently JEDEC defines:

Value Nominal Impedance Approx. capability
relative to type 0

0 50 ohm x1
1 33 ohm x1.5
2 66 ohm x0.75
3 100 ohm x0.5
4 40 ohm x1.2

For SD/SDIO, Driver Strength is an optional setting for the
UHS-I transfer modes.

The SD Association defines:

Driver Value Nominal Impedance Approx. capability
Type relative to type 0

A 1 33 ohm x1.5
B 0 50 ohm x1
C 2 66 ohm x0.75
D 3 100 ohm x0.5

So the values are the same with the exception that eMMCs have the
additional value 4 (40 ohm). It is assumed that the values will
anyway not conflict because eMMC is not removable.

Questions
-

Question 1. Should there be separate Driver Strength values for
cards and host controllers?

Originally, I was just proposing driver-strength for the
Driver Strength of the card, but now will separate this into
card-driver-strength and host-driver-strength.

Hi Adrian,

I am not sure if it makes sense to have a 'card-driver-strength'
specification for the host controller. I have been under the
impression that the proper value for this is strongly depending
on the card used in the slot. For this reason and the fact that
it is programmed in our device we added brcm,drive-strength
property in our device-tree bindings [1].

So brcm,drive-strength is drive strength used for SDIO pins on device
in mA
(default = 6) and it is written to a brcm register rather than the SDIO
CCCR Driver Strength register. So it is not in conflict, but an example of
a hardware-specific way of doing things.

I agree when you have a brcm value to write to a brcm register, it makes
sense to bind it to the brcm device.

Glad to hear. I guess I was wondering how the card-driver-strength
property would be used and by what entity, but that is not directly
relevant for having a definition for it in ACPI/devicetree.

Regards,
Arend

One problem with having the standard property bind to the card is that
removable cards are not usually represented.

Regards,
Arend

[1]
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt

Adrian, Arend,

My experience is that the value also depends very much on the board
design as well as the eMMC/sdio chip being used. eMMC chips have
some variation. A DT entry does make sense for eMMC and other wired in
devices but in this case the value is a property of the hardware
and burnt in as a property of the board design.

So it sounds like binding

Re: [PATCH 5/5] ARM: dts: exynos5250-snow: Enable wifi power-on

2015-01-28 Thread Arend van Spriel


On 01/28/15 11:10, Javier Martinez Canillas wrote:

The Snow board has a MMC/SDIO wifi chip that is always powered but it
needs a power sequence involving a reset (active low) and an enable
(active high) pins. Both pins are marked as active low since the MMC
simple power sequence driver asserts the pins prior to the card power
up procedure and de-asserts the pins after the card has been powered.

So the reset line will be left de-asserted and the enable pin will be
left asserted.

The chip also needs an external 32kHz reference clock to be operational
that is by the MAX77686 PMIC clock.

Add a simple MMC power sequence provider for the wifi MMC/SDIO slot.

Signed-off-by: Javier Martinez Canillasjavier.marti...@collabora.co.uk
---
  arch/arm/boot/dts/exynos5250-snow.dts | 25 -
  1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos5250-snow.dts 
b/arch/arm/boot/dts/exynos5250-snow.dts
index b9aeec430527..0f7971ba8238 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -229,6 +229,13 @@
power-supply =fet6;
backlight =backlight;
};
+
+   mmc3_pwrseq: mmc3_pwrseq {
+   reset-gpios =gpx0 2 GPIO_ACTIVE_LOW, /* WIFI_RSTn */
+   gpx0 1 GPIO_ACTIVE_LOW; /* WIFI_EN */
+   clocks =max77686 MAX77686_CLK_PMIC;
+   clock-names = ext_clock;
+   };
  };

  dp {
@@ -531,17 +538,33 @@
status = okay;
num-slots =1;
broken-cd;
+   cap-sdio-irq;


This seems like an unrelated change, right?

Regards,
Arend


card-detect-delay =200;
samsung,dw-mshc-ciu-div =3;
samsung,dw-mshc-sdr-timing =2 3;
samsung,dw-mshc-ddr-timing =1 2;
pinctrl-names = default;
-   pinctrl-0 =sd3_clksd3_cmdsd3_bus4;
+   pinctrl-0 =sd3_clksd3_cmdsd3_bus4wifi_enwifi_rst;
bus-width =4;
cap-sd-highspeed;
+   mmc-pwrseq =mmc3_pwrseq;
  };

  pinctrl_0 {
+   wifi_en: wifi-en {
+   samsung,pins = gpx0-1;
+   samsung,pin-function =1;
+   samsung,pin-pud =0;
+   samsung,pin-drv =0;
+   };
+
+   wifi_rst: wifi-rst {
+   samsung,pins = gpx0-2;
+   samsung,pin-function =1;
+   samsung,pin-pud =0;
+   samsung,pin-drv =0;
+   };
+
power_key_irq: power-key-irq {
samsung,pins = gpx1-3;
samsung,pin-function =0xf;


--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/3] i2c: iproc: Add Broadcom iProc I2C Driver

2015-01-18 Thread Arend van Spriel


On 01/18/15 12:17, Uwe Kleine-König wrote:

Hello Wolfram,

On Sun, Jan 18, 2015 at 12:06:58PM +0100, Wolfram Sang wrote:

On Sun, Jan 18, 2015 at 10:47:41AM +0100, Uwe Kleine-König wrote:

On Sun, Jan 18, 2015 at 10:14:04AM +0100, Arend van Spriel wrote:

On 01/17/15 00:42, Ray Jui wrote:

+   complete_all(iproc_i2c-done);


Looking over this code it seems to me there is always a single
process waiting for iproc_i2c-done to complete. So using complete()
here would suffice.

Yeah, there is always only a single thread waiting. That means both
complete and complete_all are suitable. AFAIK there is no reason to pick
one over the other in this case.


Clarity?

And which do you consider more clear? complete_all might result in the
question: Is there1 waiter? and complete might yield to What about
the other waiters?. If you already know there is only one, both are on
par on clarity. Might only be me?! I don't care much.


Maybe it is me, but it is not about questions but it is about implicit 
statements that the code makes (or reader derives from it). When using 
complete_all you indicate to the reader there can be more than one 
waiter. When using complete it indicates there is only one waiter. If 
those statements are not true that is a code issue/bug.


Regards,
Arend


Best regards
Uwe



--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/3] i2c: iproc: Add Broadcom iProc I2C Driver

2015-01-18 Thread Arend van Spriel


On 01/18/15 12:56, Uwe Kleine-König wrote:

Hello,

On Sun, Jan 18, 2015 at 12:46:51PM +0100, Arend van Spriel wrote:

On 01/18/15 12:17, Uwe Kleine-König wrote:

Hello Wolfram,

On Sun, Jan 18, 2015 at 12:06:58PM +0100, Wolfram Sang wrote:

On Sun, Jan 18, 2015 at 10:47:41AM +0100, Uwe Kleine-König wrote:

On Sun, Jan 18, 2015 at 10:14:04AM +0100, Arend van Spriel wrote:

On 01/17/15 00:42, Ray Jui wrote:

+   complete_all(iproc_i2c-done);


Looking over this code it seems to me there is always a single
process waiting for iproc_i2c-done to complete. So using complete()
here would suffice.

Yeah, there is always only a single thread waiting. That means both
complete and complete_all are suitable. AFAIK there is no reason to pick
one over the other in this case.


Clarity?

And which do you consider more clear? complete_all might result in the
question: Is there1 waiter? and complete might yield to What about
the other waiters?. If you already know there is only one, both are on
par on clarity. Might only be me?! I don't care much.


Maybe it is me, but it is not about questions but it is about
implicit statements that the code makes (or reader derives from it).
When using complete_all you indicate to the reader there can be
more than one waiter. When using complete it indicates there is
only one waiter. If those statements are not true that is a code

No, complete works just fine in the presence of1 waiter. It just wakes
a single waiter and all others continue to wait.


Yes. Agree.


That is, for single-waiter situations there is no semantic difference
between complete and complete_all. But there is a difference for
multi-waiter queues.


Indeed.


I think this is just a matter of your POV in the single-waiter
situation: complete might be intuitive because you just completed a
single task and complete_all might be intuitive because it signals
I'm completely done, there is noone waiting for me any more..


Ok. Let's leave it to the author's intuition or to say it differently 
sorry for the noise ;-)


Regards,
Arend


Best regards
Uwe



--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/3] i2c: iproc: Add Broadcom iProc I2C Driver

2015-01-18 Thread Arend van Spriel


On 01/17/15 00:42, Ray Jui wrote:

[...]


+/*
+ * Can be expanded in the future if more interrupt status bits are utilized
+ */
+#define ISR_MASK (1  IS_M_START_BUSY_SHIFT)
+
+static irqreturn_t bcm_iproc_i2c_isr(int irq, void *data)
+{
+   struct bcm_iproc_i2c_dev *iproc_i2c = data;
+   u32 status = readl(iproc_i2c-base + IS_OFFSET);
+
+   status= ISR_MASK;
+
+   if (!status)
+   return IRQ_NONE;
+
+   writel(status, iproc_i2c-base + IS_OFFSET);
+   complete_all(iproc_i2c-done);


Looking over this code it seems to me there is always a single process 
waiting for iproc_i2c-done to complete. So using complete() here would 
suffice.


Regards,
Arend


+
+   return IRQ_HANDLED;
+}


--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch v5 2/2] gpio: Document GPIO hogging mechanism

2015-01-12 Thread Arend van Spriel


On 01/12/15 11:20, Linus Walleij wrote:

On Fri, Dec 19, 2014 at 9:07 PM, Benoit Parrotbpar...@ti.com  wrote:


Add GPIO hogging documentation to gpio.txt

Signed-off-by: Benoit Parrotbpar...@ti.com
Reviewed-by: Alexandre Courbotacour...@nvidia.com


This is starting to look good ...


+   line_b {
+   gpio-hog;
+   gpios =6 0;
+   state = output-low;


I don't like the state string.

Instead have boolean properties for all states.

line_b {
 gpio-hog;
 gpios =6 0;
 output-low;
 line-name = foo-bar-gpio;
}

Then use of_property_read_bool() in the code to check which
state is to be selected intially. You can check that no mutually
exclusive state are selected, I don't like that an arbitrary string
select the state like that, if we do it that way an enumerator would
be better, I prefer bools.


To avoid the mutual exclusive state checking, would it not be more 
straightforward to use numeric enum values defined in boot/dts/include.


Regards,
Arend


Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] pci: iProc: define Broadcom iProc PCIe binding

2014-12-14 Thread Arend van Spriel


On 12/13/14 20:46, Arnd Bergmann wrote:

On Saturday 13 December 2014 11:05:52 Arend van Spriel wrote:


Makes sense. I think that is what Hauke meant by adding
additional support for registering to bcma. So the discovery info is a
piece of read-only memory in the chip. Its address is stored in the
chipcommon core register space. BCMA parses that memory blob resulting
in a list of cores which register address info. We could add DT support
in BCMA matching the compatible string and register a core for it.


Ah, interesting idea. That would mirror what we do for drivers/amba,
I like the idea.


+ Rafal

Let's explore this. Although I don't have the iProc hardware to verify it.


However, apart from the discovery info a discoverable ARM AXI chip has
a register space per core that provides common procedures like
enable/disable, reset, core status, which are implemented in BCMA. I am
not seeing that register space in the DT examples so I guess this IP
block is not there for iProc chips.


I wouldn't draw conclusions from the absence of some node. Maybe these
registers are present but just not used by the original BSP.


I do not intend to. We have raised the question internally to iProc chip 
designers.


Regards,
Arend
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] pci: iProc: define Broadcom iProc PCIe binding

2014-12-13 Thread Arend van Spriel


On 12/12/14 18:14, Arnd Bergmann wrote:

On Friday 12 December 2014 08:53:44 Ray Jui wrote:


On 12/12/2014 4:14 AM, Arnd Bergmann wrote:

On Thursday 11 December 2014 18:36:54 Ray Jui wrote:

index 000..040bc0f
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
@@ -0,0 +1,74 @@
+* Broadcom iProc PCIe controller
+
+Required properties:
+- compatible: Must be brcm,iproc-pcie
+- reg: base address and length of the PCIe controller and the MDIO interface
+  that controls the PCIe PHY
+- #interrupt-cells: set to1
+- interrupts: interrupt IDs


How many, and what are they?


Different iProc SoCs might have different number of interrupts. I'll
elaborate more on the next patch.


Ok.


+- interrupt-map-mask and interrupt-map, standard PCI properties to define the
+  mapping of the PCIe interface to interrupt numbers
+- bus-range: PCI bus numbers covered
+- #address-cells: set to3
+- #size-cells: set to2
+- device_type: set to pci
+- ranges: ranges for the PCI memory and I/O regions
+- phy-addr: MDC/MDIO adddress of the PCIe PHY


It looks like the phy controller is separate from the PCI controller,
and you even list the same register range for both PHYs. Better make
that a separate driver and put the phy address into the phys reference.


Okay. In this case, I need to create a separate PHY driver under the
drivers/phy directory and have the PCIe host driver reference it through
the standard PHY API.


Yes, that is what I meant. In particular, that has the advantage of letting
you reuse the two drivers separately if some new SoC comes up that uses
one but not the other. A lot of PHY implementations can support multiple
protocols (e.g. pcie and usb3), but I don't know if yours does.


+- have-msi-inten-reg: Required for legacy iProc PCIe controllers that need the
+  MSI interrupt enable register to be set explicitly
+
+The Broadcom iProc PCie driver adapts the multi-domain structure, i.e., each
+interface has its own domain and therefore has its own device node
+Example:
+
+SoC specific DT Entry:
+
+   pcie0: pcie@18012000 {
+   compatible = brcm,iproc-pcie;
+   reg =0x18012000 0x1000,
+0x18002000 0x1000;


I guess the addresses should be relative to the BCMA bus, and this node
get moved under that. Please see Hauke's patch series, we've discussed
this in great length already.



As Arend van Spriel pointed out in the previous discussion:

BCMA core is the bus driver for discoverable ARM AXI interconnect. Apart
from that it also provides drivers for some cores. For the chips to be
discoverable it needs additional IP logic.

Not all iProc family of SoCs have the additional IP logic and for those
which don't, they cannot use the BCMA bus.


Ok, but the one from your example almost certainly does because the
addresses are exactly the same ones as on bcm53xx.

The same problem likely occurs on other peripherals, not just PCI,
so we will have to come up with a way to have a common driver for
bcma_bus and platform_bus for USB, SPI, brcmsmac, and likely others
too.


Makes sense. I think that is what Hauke meant by adding
additional support for registering to bcma. So the discovery info is a 
piece of read-only memory in the chip. Its address is stored in the 
chipcommon core register space. BCMA parses that memory blob resulting 
in a list of cores which register address info. We could add DT support 
in BCMA matching the compatible string and register a core for it.


However, apart from the discovery info a discoverable ARM AXI chip has 
a register space per core that provides common procedures like 
enable/disable, reset, core status, which are implemented in BCMA. I am 
not seeing that register space in the DT examples so I guess this IP 
block is not there for iProc chips.


Regards,
Arend


+   #interrupt-cells =1;
+   interrupts =GIC_SPI 96 IRQ_TYPE_NONE,
+GIC_SPI 97 IRQ_TYPE_NONE,
+GIC_SPI 98 IRQ_TYPE_NONE,
+GIC_SPI 99 IRQ_TYPE_NONE,
+GIC_SPI 100 IRQ_TYPE_NONE,
+GIC_SPI 101 IRQ_TYPE_NONE;





+   interrupt-map-mask =0 0 0 0;
+   interrupt-map =0 0 0 0gic GIC_SPI 100 IRQ_TYPE_NONE;


This interrupt is also listed in the interrupts above, which is
probably a mistake, unless the IRQ line is shared between all PCI
devices and the PCI host itself.


interrupts are for MSI interrupt support and interrupt-map is for legacy
INTx support. To my best knowledge, MSI and INTx cannot be used at the
same time. nvidia,tegra20-pcie.txt and rcar-pci.txt have similar
configurations.


Linux drivers will absolutely use MSI and legacy interrupts together, because
some drivers don't support MSI and others enable it unconditionally.

In both your examples (tegra and rcar), the interrupts that share the same
number are auxiliary and are correctly used with IRQF_SHARED, so that works.
If a device MSI just maps to a host IRQ however, you wouldn't be able to
use IRQF_SHARED.

Arnd
--
To unsubscribe from this list: send

Re: [PATCH 2/4] PCI: iproc: Add Broadcom iProc PCIe driver

2014-12-11 Thread Arend van Spriel


+ Rafal

On 12/10/14 19:46, Florian Fainelli wrote:

2014-12-10 8:46 GMT-08:00 Scott Brandensbran...@broadcom.com:

On 14-12-10 03:31 AM, Arnd Bergmann wrote:


On Tuesday 09 December 2014 16:04:29 Ray Jui wrote:


Add initial version of the Broadcom iProc PCIe driver. This driver
has been tested on NSP and Cygnus and is expected to work on all iProc
family of SoCs that deploys the same PCIe host controller

The driver also supports MSI

Signed-off-by: Ray Juir...@broadcom.com
Reviewed-by: Scott Brandensbran...@broadcom.com



The driver looks suspiciously like the one that Hauke already submitted a
while ago for bcm53xx. Please come up with a merged driver that works for
both.


Could you please be a little more specific.  What driver did Hauke already
submitted?  I do not see any driver in the kernel you are talking about.


https://www.marc.info/?l=linux-pcim=141547043110684w=2




Are you sure that iProc isn't based on the BCMA bus infrastructure after
all? Even the physical address of your PCI host falls into the address
range that is used for the internal BCMA bus on the other chips!


BCMA seems to be for MIPS architectures.  It seems to be quite specific to
those architectures using BCMA.  I see no use of it in bcm53xx code?


BCMA lives in its own directory in drivers/bcma/ and is not specific
to MIPS actually. Older BCM47xx/BCM53xx MIPS-based SoCs traditionally
started with a discoverable Silicon Sonics Backplane (drivers/ssb) and
progressively migrated to BCMA (drivers/bcma), both subsystems offer a
very similar bus/device/driver abstraction and discovery mechanism.


BCMA core is the bus driver for discoverable ARM AXI interconnect. Apart 
from that it also provides drivers for some cores. For the chips to be 
discoverable it needs additional IP logic. If that is not used in the 
iProc family devices, it can not use the BCMA-based PCIe controller 
driver that Hauke submitted unless BCMA would provide an API to provide 
the chips' core information statically either per core or a full list.


Regards,
Arend
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] bcma: register bcma as device tree driver

2014-09-18 Thread Arend van Spriel


On 09/17/14 17:10, Rafał Miłecki wrote:

On 16 September 2014 23:56, Hauke Mehrtensha...@hauke-m.de  wrote:

This driver is used by the bcm53xx ARM SoC code. Now it is possible to
give the address of the chipcommon core in device tree and bcma will
search for all the other cores.


Did you get any answer from Arend about detecting IRQs?

If not:
Arend: how much time it make take you to get an answer?


I already discussed this with our chip architect. There is a chip design 
rule that core index corresponds to the irq number. This at least is 
true for the bcm43xx devices, but unsure about others. So that option 
does not seem to fit. That leaves the OOB_ROUTER core I mentioned 
earlier, but it is a chip designers nightmare as they told me. Did not 
sound encouraging enough to start digging in there to obtain the 
information.


Regards,
Arend
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] bcma: register bcma as device tree driver

2014-09-13 Thread Arend van Spriel


On 09/13/14 15:37, Hauke Mehrtens wrote:

This driver is used by the bcm53xx ARM SoC code. Now it is possible to
give the address of the chipcommon core in device tree and bcma will
search for all the other cores.

Signed-off-by: Hauke Mehrtensha...@hauke-m.de
---
  Documentation/devicetree/bindings/bus/bcma.txt | 41 +
  drivers/bcma/bcma_private.h| 16 +
  drivers/bcma/host_soc.c| 82 ++
  drivers/bcma/main.c| 10 
  include/linux/bcma/bcma.h  |  2 +
  5 files changed, 151 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/bus/bcma.txt

This is based on wireless-testing and should go into that tree.

changes since:
RFC:
  - reworded the irq description
  - improved the example
  - hocked into bcma_modeinit() and bcma_modexit()

diff --git a/Documentation/devicetree/bindings/bus/bcma.txt 
b/Documentation/devicetree/bindings/bus/bcma.txt
new file mode 100644
index 000..17e095f
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/bcma.txt
@@ -0,0 +1,41 @@
+Broadcom AIX SoC bcma bus driver


Hi Hauke,

First of all a typo used all over the place: AIX should be AXI.

The backplane in Broadcom SoC is ARM AXI with additional plugin option 
to make it discoverable. Indeed the IRQ info is not included, but I see 
no reason for specifying the register space for the cores in device-tree 
as that is discoverable by bcma.


Regards,
Arend
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] bcma: register bcma as device tree driver

2014-09-13 Thread Arend van Spriel


On 09/13/14 18:02, Hauke Mehrtens wrote:

On 09/13/2014 05:13 PM, Arend van Spriel wrote:

On 09/13/14 15:37, Hauke Mehrtens wrote:

This driver is used by the bcm53xx ARM SoC code. Now it is possible to
give the address of the chipcommon core in device tree and bcma will
search for all the other cores.

Signed-off-by: Hauke Mehrtensha...@hauke-m.de
---
   Documentation/devicetree/bindings/bus/bcma.txt | 41 +
   drivers/bcma/bcma_private.h| 16 +
   drivers/bcma/host_soc.c| 82
++
   drivers/bcma/main.c| 10 
   include/linux/bcma/bcma.h  |  2 +
   5 files changed, 151 insertions(+)
   create mode 100644 Documentation/devicetree/bindings/bus/bcma.txt

This is based on wireless-testing and should go into that tree.

changes since:
RFC:
   - reworded the irq description
   - improved the example
   - hocked into bcma_modeinit() and bcma_modexit()

diff --git a/Documentation/devicetree/bindings/bus/bcma.txt
b/Documentation/devicetree/bindings/bus/bcma.txt
new file mode 100644
index 000..17e095f
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/bcma.txt
@@ -0,0 +1,41 @@
+Broadcom AIX SoC bcma bus driver


Hi Hauke,

First of all a typo used all over the place: AIX should be AXI.


I will fix that.


The backplane in Broadcom SoC is ARM AXI with additional plugin option
to make it discoverable. Indeed the IRQ info is not included, but I see
no reason for specifying the register space for the cores in device-tree
as that is discoverable by bcma.


I specified the register space to make it possible to connect the device
tree entry with the core. After the cores are automatically discovered,
bcma searches for entry core found, for an device tree entry with the
same address space and uses the irq number from that entry. If there is
a core defined in device tree which is not found by bcma it will just be
ignored. If a core is not specified in device tree it will get
registered, but it will not get an IRQ.
This was the most stable way I came up with, I also thought about using
the core number, like assign the this IRQ to core number 5, but the core
numbers could change when we do changes to bcma.


I understand. In the example I noticed the core base address was also 
used in the entry label, ie. pcie@18002000. However, I am not sure 
whether that can be obtained by the driver. If it is possible it could 
be used to match the dt entry to a core and the register info would be 
redundant.



In the Broadcom vendor code there is a list with the IRQ numbers which
get assigned to a specific core type. For example there is a list with 4
IRQ numbers which get assigned to the first 4 Ethernet cores. This would
also work, but I do not know how to do this in device tree.


I am wondering about the IRQ numbers. The SoC would also have a OOB 
routing core, which controls non-axi signals between the cores. I will 
ask internally whether the irq signals are controlled by it and can be 
retrieved from it.


Regards,
Arend
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/2] pwrseq: Add subsystem to handle complex power sequences

2014-07-01 Thread Arend van Spriel

On 01-07-14 18:42, Ulf Hansson wrote:
 On 20 June 2014 10:14, Hans de Goede hdego...@redhat.com wrote:
 Hi,

 On 06/20/2014 10:02 AM, Olof Johansson wrote:
 On Fri, Jun 20, 2014 at 12:27 AM, Hans de Goede hdego...@redhat.com wrote:
 Hi,

 On 06/19/2014 07:18 PM, Olof Johansson wrote:
 Hi,



 On Thu, Jun 19, 2014 at 6:04 AM, Ulf Hansson ulf.hans...@linaro.org 
 wrote:
 The pwrseq subsystem handles complex power sequences, typically useful
 for subsystems that makes use of discoverable buses, like for example
 MMC and I2C.

 The pwrseq subsystem is dependant on CONFIG_OF to be able to parse a DT
 childnode to find out what power sequence method to bind for a device.

 From the DT childnode, the pwrseq DT parser tries to locate a
 power-method property, which string is matched towards the list of
 supported pwrseq methods.

 Each pwrseq method implements it's own power sequence and interfaces
 the pwrseq core through a few callback functions.

 To instantiate a pwrseq method, clients shall use the devm_pwrseq_get()
 API. If needed, clients can explicity drop the references to a pwrseq
 method using devm_pwrseq_put() API.

 Besides instantiation, the pwrseq API provides clients opportunity to
 select a certain power state. In this intial version, PWRSEQ_POWER_ON
 and PWRSEQ_POWER_OFF are supported. Those are also mandatory for each
 pwrseq method to support.

 Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
 ---
  .../devicetree/bindings/pwrseq/pwrseq.txt  |   48 ++
  drivers/Makefile   |2 +-
  drivers/pwrseq/Makefile|2 +
  drivers/pwrseq/core.c  |  175 
 
  drivers/pwrseq/core.h  |   37 +
  drivers/pwrseq/method.c|   38 +
  include/linux/pwrseq.h |   50 ++
  7 files changed, 351 insertions(+), 1 deletion(-)
  create mode 100644 Documentation/devicetree/bindings/pwrseq/pwrseq.txt
  create mode 100644 drivers/pwrseq/Makefile
  create mode 100644 drivers/pwrseq/core.c
  create mode 100644 drivers/pwrseq/core.h
  create mode 100644 drivers/pwrseq/method.c
  create mode 100644 include/linux/pwrseq.h

 diff --git a/Documentation/devicetree/bindings/pwrseq/pwrseq.txt 
 b/Documentation/devicetree/bindings/pwrseq/pwrseq.txt
 new file mode 100644
 index 000..80848ae
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/pwrseq/pwrseq.txt
 @@ -0,0 +1,48 @@
 +Power sequence DT bindings
 +
 +Each power sequence method has a corresponding power-method property 
 string.
 +This property shall be set in a subnode for a device. That subnode 
 should also
 +describe resourses which are specific to that power method.
 +
 +Do note, power sequences as such isn't encoded through DT. Instead 
 those are
 +implemented by each power method.
 +
 +Required subnode properties:
 +- power-method: should contain the string for the power method to bind.
 +
 +   Supported power methods: None.
 +
 +Example:
 +
 +Note, the clock power method in this example isn't actually 
 supported, but
 +used to visualize how a childnode could be described.
 +
 +// WLAN SDIO channel
 +sdi1_per2@80118000 {
 +   compatible = arm,pl18x, arm,primecell;
 +   reg = 0x80118000 0x1000;
 +   interrupts = 0 50 IRQ_TYPE_LEVEL_HIGH;
 +
 +   dmas = dma 32 0 0x2, /* Logical - DevToMem */
 +  dma 32 0 0x0; /* Logical - MemToDev */
 +   dma-names = rx, tx;
 +
 +   clocks = prcc_kclk 2 4, prcc_pclk 2 6;
 +   clock-names = sdi, apb_pclk;
 +
 +   arm,primecell-periphid = 0x10480180;
 +   max-frequency = 1;
 +   bus-width = 4;
 +   non-removable;
 +   pinctrl-names = default, sleep;
 +   pinctrl-0 = sdi1_default_mode;
 +   pinctrl-1 = sdi1_sleep_mode;
 +
 +   status = okay;
 +
 +   pwrseq: pwrseq1 {
 +   power-method = clock;
 +   clocks = someclk 1 2, someclk 3 4;
 +   clock-names = pwrseq1, pwrseq2;
 +   };

 I am strongly against the subnode approach as a general framework. We
 don't have a subnode for interrupts, nor for clocks or pinctrl. So why
 should we have it for the power sequencing?

 Sure, that fits the linux driver model better, but that's irrelevant
 w.r.t. describing the hardware.

 Actually this is about describing the hardware, when you have e.g. an
 mmc device which needs pwrseq, there will be 2 sets of certain
 resources, ie clocks for the host controller and clocks going directly
 to the mmc device. I think putting those both in the same subnode is
 a BAD idea, so we really do need a subnode to group the pwrseq resources
 together.

 I disagree.

 The clock is the input to the module, and it is what needs to be
 enabled for the module to work. It's not the input to some
 power-sequence component on the module, or next to the module on the
 bus.

 Right, it is an input to the sdio-module, not to the

Re: [PATCH v3 1/4] dt: bindings: add bindings for Broadcom bcm43xx sdio devices

2014-06-30 Thread Arend van Spriel

On 29-06-14 16:16, Hans de Goede wrote:
 From: Arend van Spriel ar...@broadcom.com
 
 The Broadcom bcm43xx sdio devices are fullmac devices that may be
 integrated in ARM platforms. Currently, the brcmfmac driver for
 these devices support use of platform data. This patch specifies
 the bindings that allow this platform data to be expressed in the
 devicetree.
 
 Reviewed-by: Hante Meuleman meule...@broadcom.com
 Reviewed-by: Franky (Zhenhui) Lin fran...@broadcom.com
 Reviewed-by: Daniel (Deognyoun) Kim de...@broadcom.com
 Reviewed-by: Pieter-Paul Giesberts piete...@broadcom.com
 Signed-off-by: Arend van Spriel ar...@broadcom.com
 [hdego...@redhat.com: drop clk / reg_on gpio handling, as there is no 
 consensus
  on how to handle this yet]
 [hdego...@redhat.com: move from bindings/staging to bindings]
 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
  .../bindings/net/wireless/brcm,bcm43xx-fmac.txt| 41 
 ++
  1 file changed, 41 insertions(+)
  create mode 100644 
 Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
 
 diff --git 
 a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt 
 b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
 new file mode 100644
 index 000..5dbf169
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
 @@ -0,0 +1,41 @@
 +Broadcom BCM43xx Fullmac wireless SDIO devices
 +
 +This node provides properties for controlling the Broadcom wireless device. 
 The
 +node is expected to be specified as a child node to the SDIO controller that
 +connects the device to the system.
 +
 +Required properties:
 +
 + - compatible : Should be brcm,bcm4329-fmac.
 +
 +Optional properties:
 + - brcm,drive-strength : drive strength used for SDIO pins on device in mA
 + (default = 6).
 + - interrupt-parent : the phandle for the interrupt controller to which the
 + device interrupts are connected.
 + - interrupts : specifies attributes for the out-of-band interrupt 
 (host-wake).
 + When not specified the device will use in-band SDIO interrupts.
 + - interrupt-names : name of the out-of-band interrupt, which must be set
 + to host-wake.
 +
 +Example:
 +
 +mmc3: mmc@01c12000 {
 + #address-cells = 1;
 + #size-cells = 0;
 +
 + pinctrl-names = default;
 + pinctrl-0 = mmc3_pins_a;
 + vmmc-supply = reg_vmmc3;
 + bus-width = 4;
 + non-removable;
 + status = okay;
 +
 + brcmf: bcrmf@1 {
 + reg = 1;

I guess the reg property is needed to have this node linked to sdio func
1 dev, right? I would add it to the list of required properties above
even if this behaviour is already described in a more generic binding.

Regards,
Arend

 + compatible = brcm,bcm4329-fmac;
 + interrupt-parent = pio;
 + interrupts = 10 8; /* PH10 / EINT10 */
 + interrupt-names = host-wake;
 + };
 +};
 

--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/4] brcmfmac: Fix OOB interrupt not working for BCM43362

2014-06-18 Thread Arend van Spriel


On 16-06-14 19:56, Hans de Goede wrote:

It has taken me a long long time to get the OOB interrupt working on the
AP6210 sdio wifi/bt module found on various Allwinner A20 boards. In the
end I found these magic register pokes in the cubietruck kernel tree:
https://github.com/cubieboard2/linux-sunxi/commit/7f08ba395617d17e7a711507503d89a50406fe7a

I'm not entirely sure if this specific to the AP6210 module, or if this
should be done for all BCM43362 sdio devices.


Bit late response but it took some digging. It turns out this is done 
for all bcm43362 in internal/proprietary driver.


+Reviewed-by: Arend van Spriel ar...@broadcom.com

Signed-off-by: Hans de Goede hdego...@redhat.com
---
  drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 16 
  1 file changed, 16 insertions(+)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c 
b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 0fc707c..58e86a5 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -39,7 +39,9 @@
  #include brcm_hw_ids.h
  #include brcmu_utils.h
  #include brcmu_wifi.h
+#include chipcommon.h
  #include soc.h
+#include chip.h


Not sure why chip.h is needed here.


  #include dhd_bus.h
  #include dhd_dbg.h
  #include sdio_host.h
@@ -119,6 +121,7 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev 
*sdiodev)
  {
int ret = 0;
u8 data;
+   u32 addr, gpiocontrol;
unsigned long flags;

if ((sdiodev-pdata)  (sdiodev-pdata-oob_irq_supported)) {
@@ -148,6 +151,19 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev 
*sdiodev)

sdio_claim_host(sdiodev-func[1]);

+   if (sdiodev-bus_if-chip == BCM43362_CHIP_ID) {
+   /* assign GPIO to SDIO core */
+   addr = CORE_CC_REG(SI_ENUM_BASE, gpiocontrol);


I don't like the assumption that chipcommon core register space is 
always at SI_ENUM_BASE although it is for BCM43362. Let's keep it for now.



+   gpiocontrol = brcmf_sdiod_regrl(sdiodev, addr, ret);
+   gpiocontrol |= 0x2;
+   brcmf_sdiod_regwl(sdiodev, addr, gpiocontrol, ret);
+
+   brcmf_sdiod_regwb(sdiodev, SBSDIO_GPIO_SELECT, 0xf,
+ ret);
+   brcmf_sdiod_regwb(sdiodev, SBSDIO_GPIO_OUT, 0, ret);
+   brcmf_sdiod_regwb(sdiodev, SBSDIO_GPIO_EN, 0x2, ret);
+   }
+
/* must configure SDIO_CCCR_IENx to enable irq */
data = brcmf_sdiod_regrb(sdiodev, SDIO_CCCR_IENx, ret);
data |= 1  SDIO_FUNC_1 | 1  SDIO_FUNC_2 | 1;



Regards,
Arend
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] dt: bindings: add bindings for Broadcom bcm43xx sdio devices

2014-06-17 Thread Arend van Spriel


On 17-06-14 08:32, Hans de Goede wrote:

Hi,

On 06/16/2014 10:53 PM, Florian Fainelli wrote:

2014-06-16 10:56 GMT-07:00 Hans de Goede hdego...@redhat.com:

From: Arend van Spriel ar...@broadcom.com

The Broadcom bcm43xx sdio devices are fullmac devices that may be
integrated in ARM platforms. Currently, the brcmfmac driver for
these devices support use of platform data. This patch specifies
the bindings that allow this platform data to be expressed in the
devicetree.

Reviewed-by: Hante Meuleman meule...@broadcom.com
Reviewed-by: Franky (Zhenhui) Lin fran...@broadcom.com
Reviewed-by: Daniel (Deognyoun) Kim de...@broadcom.com
Reviewed-by: Pieter-Paul Giesberts piete...@broadcom.com
Signed-off-by: Arend van Spriel ar...@broadcom.com
[hdego...@redhat.com: drop clk / reg_on gpio handling, as there is no consensus
  on how to handle this yet]
[hdego...@redhat.com: move from bindings/staging to bindings]
Signed-off-by: Hans de Goede hdego...@redhat.com
---
  .../bindings/net/wireless/brcm,bcm43xx-fmac.txt| 29 ++
  1 file changed, 29 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt

diff --git 
a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt 
b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
new file mode 100644
index 000..6a0aaf2
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
@@ -0,0 +1,29 @@
+Broadcom BCM43xx Fullmac wireless SDIO devices
+
+This node provides properties for controlling the Broadcom wireless device. The
+node is expected to be specified as a child node to the SDIO controller that
+connects the device to the system.
+
+Required properties:
+
+ - compatible : Should be brcm,bcm43xx-fmac.


In general, the use of a wildcard compatible string is discouraged
over the use of a more descriptive compatible string. So you should
find out what is the first chip that is compatible, and use that
compatible string as long as that compatibility remains.


Right, Arend, what should we use then ?


In earlier discussions, we ended up with this compatible string. The 
properties are generic enough to be covered by this 'wildcard' string. 
However, I am not religious about it so if you feel strongly for an 
explicit string it could be brcm,bcm4329-fmac, but it does not have my 
preference.



+
+Optional properties:
+ - brcm,drive-strength : drive strength used for SDIO pins on device.
+   (default = 6mA).


It would not hurt if you did specify the unit of the
brcm,drive-strength property here. Also, if the default is 6, maybe
the example below should be fixed to reflect that so people do not
think that writing 4 to a register = 6 mA.


Ok will fix in the next version.



I agree that explicitly specifying the unit of the property is a good 
addition. If that is done I don't think it is necessary to change the 
value used in the example.


Regards,
Arend




+ - interrupt-parent : the phandle for the interrupt controller to which the
+   device interrupts are connected.
+ - interrupts : specifies attributes for the out-of-band interrupt (host-wake).
+   When not specified the device will use in-band SDIO interrupts.
+ - interrupt-names : name of the out-of-band interrupt, which must be set
+   to host-wake.
+
+Example:
+
+bcm4335 {
+   compatible = brcm,bcm43xx-fmac;
+   brcm,drive-strength = 4;
+   interrupt-parent = gpx2;
+   interrupts = 5 IRQ_TYPE_LEVEL_HIGH;
+   interrupt-names = host-wake;
+};
--
2.0.0


___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel






--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/4] brcmfmac: Fix OOB interrupt not working for BCM43362

2014-06-17 Thread Arend van Spriel


On 16-06-14 19:56, Hans de Goede wrote:

It has taken me a long long time to get the OOB interrupt working on the
AP6210 sdio wifi/bt module found on various Allwinner A20 boards. In the
end I found these magic register pokes in the cubietruck kernel tree:
https://github.com/cubieboard2/linux-sunxi/commit/7f08ba395617d17e7a711507503d89a50406fe7a

I'm not entirely sure if this specific to the AP6210 module, or if this
should be done for all BCM43362 sdio devices.


Hi Hans,

I was still not sure about this one so I asked around. Apart from the 
nvram settings I emailed earlier one more is needed:


sd_gpout=1
sd_oobonly=1
sd_gpval=1
*muxenab=0x11*

Could you give that a try?

Regards,
Arend


Signed-off-by: Hans de Goede hdego...@redhat.com
---
  drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 16 
  1 file changed, 16 insertions(+)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c 
b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 0fc707c..58e86a5 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -39,7 +39,9 @@
  #include brcm_hw_ids.h
  #include brcmu_utils.h
  #include brcmu_wifi.h
+#include chipcommon.h
  #include soc.h
+#include chip.h
  #include dhd_bus.h
  #include dhd_dbg.h
  #include sdio_host.h
@@ -119,6 +121,7 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev 
*sdiodev)
  {
int ret = 0;
u8 data;
+   u32 addr, gpiocontrol;
unsigned long flags;

if ((sdiodev-pdata)  (sdiodev-pdata-oob_irq_supported)) {
@@ -148,6 +151,19 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev 
*sdiodev)

sdio_claim_host(sdiodev-func[1]);

+   if (sdiodev-bus_if-chip == BCM43362_CHIP_ID) {
+   /* assign GPIO to SDIO core */
+   addr = CORE_CC_REG(SI_ENUM_BASE, gpiocontrol);
+   gpiocontrol = brcmf_sdiod_regrl(sdiodev, addr, ret);
+   gpiocontrol |= 0x2;
+   brcmf_sdiod_regwl(sdiodev, addr, gpiocontrol, ret);
+
+   brcmf_sdiod_regwb(sdiodev, SBSDIO_GPIO_SELECT, 0xf,
+ ret);
+   brcmf_sdiod_regwb(sdiodev, SBSDIO_GPIO_OUT, 0, ret);
+   brcmf_sdiod_regwb(sdiodev, SBSDIO_GPIO_EN, 0x2, ret);
+   }
+
/* must configure SDIO_CCCR_IENx to enable irq */
data = brcmf_sdiod_regrb(sdiodev, SDIO_CCCR_IENx, ret);
data |= 1  SDIO_FUNC_1 | 1  SDIO_FUNC_2 | 1;



--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/4] brcmfmac: Fix OOB interrupt not working for BCM43362

2014-06-17 Thread Arend van Spriel


On 17-06-14 16:32, Hans de Goede wrote:

Hi,

On 06/17/2014 01:49 PM, Arend van Spriel wrote:

On 16-06-14 19:56, Hans de Goede wrote:

It has taken me a long long time to get the OOB interrupt working on the
AP6210 sdio wifi/bt module found on various Allwinner A20 boards. In the
end I found these magic register pokes in the cubietruck kernel tree:
https://github.com/cubieboard2/linux-sunxi/commit/7f08ba395617d17e7a711507503d89a50406fe7a

I'm not entirely sure if this specific to the AP6210 module, or if this
should be done for all BCM43362 sdio devices.


Hi Hans,

I was still not sure about this one so I asked around. Apart from the nvram 
settings I emailed earlier one more is needed:

sd_gpout=1
sd_oobonly=1
sd_gpval=1
*muxenab=0x11*

Could you give that a try?


I seriously doubt that is going to do anything at all, all 4 these keywords do 
not
show up when running strings on brcmfmac43362-sdio.bin, where as all the
keywords used in the brcmfmac43362-sdio.txt shipped with the android on ap6210
equipped boards do show up, confirming that the keywords are encoded in plain 
text
in the firmware file.

Let me know if want me to give it a try anyways and I'll do so.

Perhaps these keywords need a newer firmware version then the one in the
linux-firmware repository ?


I will checkout the firmware release and see what is built there.

Thanks,
Arend
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/11] brcmfmac: Fix OOB interrupt not working for BCM43362

2014-05-28 Thread Arend van Spriel


On 05/27/14 23:28, Hans de Goede wrote:

Hi,

On 05/27/2014 07:03 PM, Arend van Spriel wrote:

On 05/26/14 09:48, Hans de Goede wrote:

It has taken me a long long time to get the OOB interrupt working on the
AP6210 sdio wifi/bt module found on various Allwinner A20 boards. In the
end I found these magic register pokes in the cubietruck kernel tree:
https://github.com/cubieboard2/linux-sunxi/commit/7f08ba395617d17e7a711507503d89a50406fe7a


I'm not entirely sure if this specific to the AP6210 module, or if this
should be done for all BCM43362 sdio devices.


Let keep it as this for now. I added some remarks below.

Regards,
Arend


Signed-off-by: Hans de Goedehdego...@redhat.com
---
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 18 ++
1 file changed, 18 insertions(+)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 0fc707c..2369a0f 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -39,7 +39,9 @@
#includebrcm_hw_ids.h
#includebrcmu_utils.h
#includebrcmu_wifi.h
+#includechipcommon.h
#includesoc.h
+#include chip.h
#include dhd_bus.h
#include dhd_dbg.h
#include sdio_host.h
@@ -119,6 +121,7 @@ int brcmf_sdiod_intr_register(struct
brcmf_sdio_dev *sdiodev)
{
int ret = 0;
u8 data;
+ u32 addr, gpiocontrol;
unsigned long flags;

if ((sdiodev-pdata) (sdiodev-pdata-oob_irq_supported)) {
@@ -148,6 +151,21 @@ int brcmf_sdiod_intr_register(struct
brcmf_sdio_dev *sdiodev)

sdio_claim_host(sdiodev-func[1]);

+ if (sdiodev-bus_if-chip == BCM43362_CHIP_ID) {

+ /* assign GPIO to SDIO core */

+ addr = CORE_CC_REG(SI_ENUM_BASE, gpiocontrol);
+ gpiocontrol = brcmf_sdiod_regrl(sdiodev, addr,ret);
+ gpiocontrol |= 0x2;
+ brcmf_sdiod_regwl(sdiodev, addr, gpiocontrol,ret);
+
+ /* SPROM_ADDR_HIGH ? perhaps the defines name is off */


These names are all off. These should be:
SBSDIO_SPROM_ADDR_HIGH = SBSDIO_GPIO_SELECT
SBSDIO_CHIP_CTRL_DATA = SBSDIO_GPIO_OUT
SBSDIO_CHIP_CTRL_EN = SBSDIO_GPIO_EN


Ok, so I guess I should do a pre-patch adding defines for these, should
these replace the existing defines for these addresses in:
drivers/net/wireless/brcm80211/brcmfmac/sdio_host.h

Or should the pre-patch add defines with the new names and keep the old
ones ?


These defines are not used in the code so go ahead and replace them. It 
needs some more cleanup, but I can do a subsequent patch for that.


Gr. AvS
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: representing sdio devices oob interrupt, clks, etc. in device tree

2014-05-28 Thread Arend van Spriel


On 05/28/14 11:42, Hans de Goede wrote:

Hi,

On 05/27/2014 03:50 PM, Ulf Hansson wrote:

[snip]


I am having a bit hard to follow the terminology here. :-) What is a
powerup driver and what is a main device driver in this context?

I had a slide which I used at a mmc subsystem crash course recently,
please have a look - hopefully this will help us to sort out this.

https://drive.google.com/file/d/0B2ePGK-iqMupbDQ2S0o5b3Bhek0/edit?usp=sharing


Right, the problem is that in the case(s) we're talking about before the sdio
device in question can be probed the sdio device may need to have several clk
signals enables, reset signal deasserted, etc.


Yes. That's an important point.



Some piece of code needs to do that, the proposal so far has been to let the
mmc-core deal with this, which will likely work fine for 90% of the cases
were we need any form of powerup, but in some more complex case this may need
to happen in a specific order / with specific delays, etc. In this case some
separate piece of board and/or sdio device specific code would need to take
care of this, hence I was talking about a powerup-driver, which I agree is
not the best name. So lets just forget about the power-driver nomenclature
completely.


I have now given this a serious thought, sorry for not doing this
earlier. Please consider the following ideas and statements. Feel free
to shoot them down. :-)

DT is supposed to described the hardware. Encoding power up sequences
within DT, to for example let the mmc core handle this means we are
touching the concept of open firmware. This is not what DT should be
used for.


Ok, I already expected an answer like this, but I still wanted to voice
the idea of encoding the sequence into the dt.


To describe the HW in DT, the embedded SDIO card (actually it could be
any type of embedded card) shall be modelled as a child node to the
mmc host in DT. Similar to what you have proposed, but with the
difference that the child node _must_ contain a DT compatible string,
which means a powerup-driver can be probed.

Yes, I understand we might need one DT compatible string per board,
but that's because we need to model the hardware - and it differs.

To clarify my view, we do need a powerup-driver and the primary
reason is that we must not model power up sequences within DT.
Typically I see the powerup-driver as a simple platform driver
attached to the platform bus, but I that could of course differ.


Ok, but if it is a platform device, then how can the mmc-host
depend on it ? Or will the mmc-core code instantiate this
platform device based on the child node, rather then it being
instantiated directly by the platform bus ?

snip


Well, I don't like the simple-powerup, because I think a simple
powerup sequence is to me already supported by the mmc core, through
the regular host interface (-set_ios()).

If I understand correctly, this binding is supposed to be configured
per host device and thus also per host device slots?


Yes, although I must admit that have not thought about how to deal with
slots, I've no experience with the mmc slots concept at all, or is slot
just a different name for sdio function ?


Some mmc hosts may support more than one slot. Thus they can operate
on more than one card.

Currently, there are no support for this in the mmc core. There can
only be one card per host, but that's due to software limitation of
the mmc stack.  Following your suggestion; modelling the card as child
node under the mmc host, can easily be extended to support more than
one slot.


Actually what I'm suggesting is based on Sascha Hauer's
mmc: Add SDIO function devicetree subnode parsing

Patch, which models the sdio functions as child nodes, (with the
one with reg =0  being the card itself) this also makes sense since each
sdio-function gets its own device representing it, so having one child node
per sdio-functions leads to one child node per device which seems sensible.


To complicate thing, for brcmfmac the sdio functions are not considered 
individual devices. This means that brcmfmac creates one driver instance 
which claims multiple sdio functions.


Regards,
Arend


This means how ever that if want to prepare for a future were we support
more then one slot we need an extra level for the slot, so we would
get something like this:

mmc3: mmc@01c12000 {
 #address-cells =1;
 #size-cells =0;

 pinctrl-names = default;
 pinctrl-0 =mmc3_pins_a;
 vmmc-supply =reg_vcc3v3;
 bus-width =4;
 non-removable;
 status = okay;

 mmc3slot0: mmc3slot@0 {
 #address-cells =1;
 #size-cells =0;

 reg =0;
 clocks =clk_out_a,clk_out_b;
 clock-frequency =32768,2600;
gpios =pio 4 5 0pio 1 18 0;

 brcmf: bcrmf@1 {
 reg =1;
 compatible = brcm,bcm43xx-fmac;
 interrupt-parent =pio;
 interrupts =10 4; /* PH10 / EINT10 */
 interrupt-names = host-wake;
 status =

Re: [PATCH 00/11] sdio wifi oob irq support for sunxi

2014-05-27 Thread Arend van Spriel


On 05/27/14 17:26, John W. Linville wrote:

On Mon, May 26, 2014 at 09:47:55AM +0200, Hans de Goede wrote:

Hi All,

Here is a patch series adding support for oob irqs for the ap6210 sdio wifi
modules found on various Allwinner A20 boards.

A lot of it is ground work which should be useful for adding oob irq support
to other sdio wifi models too.

The first 5 patches are sunxi pinctrl patches and should go upstream through
the pinctrl tree.

Patch 6 adds generic support for having per sdio function child nodes in
devicetree and should go upstream through the mmc tree.

Patch 7 and 8 add oob irq support to the brcmfmac driver. Patch 9 pokes some
magic bits needed to enable the oob irq, this was taken from an android code
dump and good do with a good review from someone from Broadcom, Arend ?


If Arend approves, I'm fine with these going through another appropriate tree.


Not sure in what tree these patches were created (assume some sunxi 
tree), but if it conflicts with wireless-next I am pretty sure Stephen 
will find that. I am fine with any tree. Patch 9 indeed has some magic 
bits that need some clarification, but I will sort that out with Hans.


Regards,
Arend


Patch 10 and 11 add the necessary dts bits to actually enable the sdio irq
and should go upstream through Maxime's dt tree.

Please review / merge.

Thanks  Regards,

Hans





--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/11] brcmfmac: Fix OOB interrupt not working for BCM43362

2014-05-27 Thread Arend van Spriel


On 05/26/14 09:48, Hans de Goede wrote:

It has taken me a long long time to get the OOB interrupt working on the
AP6210 sdio wifi/bt module found on various Allwinner A20 boards. In the
end I found these magic register pokes in the cubietruck kernel tree:
https://github.com/cubieboard2/linux-sunxi/commit/7f08ba395617d17e7a711507503d89a50406fe7a

I'm not entirely sure if this specific to the AP6210 module, or if this
should be done for all BCM43362 sdio devices.


Let keep it as this for now. I added some remarks below.

Regards,
Arend


Signed-off-by: Hans de Goedehdego...@redhat.com
---
  drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 18 ++
  1 file changed, 18 insertions(+)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c 
b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 0fc707c..2369a0f 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -39,7 +39,9 @@
  #includebrcm_hw_ids.h
  #includebrcmu_utils.h
  #includebrcmu_wifi.h
+#includechipcommon.h
  #includesoc.h
+#include chip.h
  #include dhd_bus.h
  #include dhd_dbg.h
  #include sdio_host.h
@@ -119,6 +121,7 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev 
*sdiodev)
  {
int ret = 0;
u8 data;
+   u32 addr, gpiocontrol;
unsigned long flags;

if ((sdiodev-pdata)  (sdiodev-pdata-oob_irq_supported)) {
@@ -148,6 +151,21 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev 
*sdiodev)

sdio_claim_host(sdiodev-func[1]);

+   if (sdiodev-bus_if-chip == BCM43362_CHIP_ID) {

+   /* assign GPIO to SDIO core */

+   addr = CORE_CC_REG(SI_ENUM_BASE, gpiocontrol);
+   gpiocontrol = brcmf_sdiod_regrl(sdiodev, addr,ret);
+   gpiocontrol |= 0x2;
+   brcmf_sdiod_regwl(sdiodev, addr, gpiocontrol,ret);
+
+   /* SPROM_ADDR_HIGH ? perhaps the defines name is off */


These names are all off. These should be:
SBSDIO_SPROM_ADDR_HIGH = SBSDIO_GPIO_SELECT
SBSDIO_CHIP_CTRL_DATA = SBSDIO_GPIO_OUT
SBSDIO_CHIP_CTRL_EN = SBSDIO_GPIO_EN


+   brcmf_sdiod_regwb(sdiodev, SBSDIO_SPROM_ADDR_HIGH, 0xf,
+   ret);
+   brcmf_sdiod_regwb(sdiodev, SBSDIO_CHIP_CTRL_DATA, 0,
+   ret);
+   brcmf_sdiod_regwb(sdiodev, SBSDIO_CHIP_CTRL_EN, 0x2,
+   ret);
+   }
+
/* must configure SDIO_CCCR_IENx to enable irq */
data = brcmf_sdiod_regrb(sdiodev, SDIO_CCCR_IENx,ret);
data |= 1  SDIO_FUNC_1 | 1  SDIO_FUNC_2 | 1;


--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: representing sdio devices oob interrupt, clks, etc. in device tree

2014-05-27 Thread Arend van Spriel


On 05/27/14 20:55, Olof Johansson wrote:

On Tue, May 27, 2014 at 06:53:26PM +0100, Mark Brown wrote:

On Tue, May 27, 2014 at 03:50:33PM +0200, Ulf Hansson wrote:


To describe the HW in DT, the embedded SDIO card (actually it could be
any type of embedded card) shall be modelled as a child node to the
mmc host in DT. Similar to what you have proposed, but with the
difference that the child node _must_ contain a DT compatible string,
which means a powerup-driver can be probed.



Yes, I understand we might need one DT compatible string per board,
but that's because we need to model the hardware - and it differs.



To clarify my view, we do need a powerup-driver and the primary
reason is that we must not model power up sequences within DT.
Typically I see the powerup-driver as a simple platform driver
attached to the platform bus, but I that could of course differ.


This then either conflicts with cases where we need to describe the
actual contents of the slot with a compatible string or means that the
SDIO driver needs to handle powerup sequencing since we should be
binding to the first compatible we find.  If the host controller driver
and/or subsystem is going to deal with the powering up it's not clear
that it specifically needs to be the compatible property that's used
to determine the powerup method, it could just be a boolean or a
'power-method = blah' property (where blah is one of a series of strings
defining methods).  Alternatively we could have separate nodes for the
slot and SDIO device but that feels meh.  What's the hard requirement
for it to specifically be a compatible property?


+1. Just because we have a subnode in a device tree, we don't have to have
a driver bind against it. The MMC core code could go down into the subnodes,
find a power-method =foo property and go ahead and parse the rest of it.
There's no requirement that we do this through the Linux driver model of
probe(), etc.


I prefer a power-method property over compatible matching. The fact that 
the subnode has a compatible string and properties for the device driver 
should not matter.



The slot will be the first level of child node under the mmc host,
then each slot may have a child node which models the embedded card.
But, let's leave that discussion for now. :-)


OK, that's the separate node for the slot and device.


Powerup driver's -probe():
Typically the powerup driver will need to register a few callback
functions towards the mmc core. Typically at mmc_of_parse(), those
callbacks will have to be connected to a particular mmc host.



I would like to see three different callbacks, mirroring each of the
mmc_ios power_mode states MMC_POWER_OFF|UP|ON.



The power up sequence, performed by the mmc core:
The mmc_power_up|off functions, will invoke the registered powerup
driver's callbacks if they exists for the particular host it operates
on.


There's also the need for the SDIO device to be able to get at the
resources provided and actively work with them at runtime if it wants to
manage things more actively (partial poweroff for low power states or
managing clock rates for example).


Again, I think it gets overly complicated by using a full driver for the
power management. Abstracted out into something separate and scalable
as number of devices grow? Sure, definitely. As a driver? Not convinced.


I think somewhere in the thread Hans already indicated the term 'driver' 
was a misnomer. While monitoring the discussion I was wondering whether 
this type of power-up sequence handling is specific to mmc/sdio or could 
it also apply to say spi, i2c, or whatever. In other words, could the 
power-up sequence code be placed in drivers/of code.


Regards,
Arend
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: representing sdio devices oob interrupt, clks, etc. in device tree

2014-05-26 Thread Arend van Spriel


+ Russell

On 05/25/14 21:20, Hans de Goede wrote:

Hi,

On 05/25/2014 02:34 PM, Mark Brown wrote:

On Sat, May 24, 2014 at 12:06:30PM +0200, Hans de Goede wrote:

On 05/23/2014 06:27 PM, Mark Brown wrote:

On Fri, May 23, 2014 at 01:50:40PM +0200, Hans de Goede wrote:

On 05/23/2014 01:22 PM, Mark Brown wrote:



The compatible is not a Linux specific thing, it is a marking saying
that something needs to take care of enabling the clks (and whatever
else we will make part of the binding for this compatible), before
scanning the mmc bus.



We could just say that the mere presence of a child node with the right
properties is sufficient to trigger the bus to do the startup?



Yes, except that most involved property names are standardized, ie clocks,
and we want to be able to opt out of the KISS mmc core code for
(future) complex power on sequences.


This is why I'm suggesting keying off the specific compatible strings
for drivers that want to opt out of the helpers rather than having a
compatible string to enable the helpers (which may depend on the
particular Linux version if the feature sets of drivers change for
example).


If the device is sufficiently complicated to need a special power up
sequence I'd expect we'd be able to have a compatible string which would
provide enough information for us to figure things out.



Hmm, so what you're suggesting is indeed more of an opt out then my initial
opt-in to KISS powerup idea. So to be clear what you're suggesting is:



mmc core walks host mmc-child nodes. Loads drivers based on compatibles
there. The checks a flag field in the driver to see if the driver wants to
opt-out of the KISS powerup code. The problem with this is that it won't


Yes.


work reliable with modules, think the mmc probe being done from the ramdisk,
and the driver in question only being available from the rootfs. I really


Why is that a problem - if we have no driver for the device there is no
point in powering it up in the first place is there?


Well the driver may show up later, so if we only do the power-up once we have
a driver, this means we need to re-check if we need to do the powerup later.

Also the mmc people are very much against specifying a driver, as that is
something which should be probed not specified. I agree with them I've
already seen boards were more or less standardized sdio modules from different
vendors are used, they have various standard sdio powerup related things, like
an enable signal in standard places, but different editions of the boards
have different sdio modules soldered on, using different drivers.

I expect that with some care we can make all variants of these boards work with
a single dts file, by using a simple sdio wifi powerup mechanism, and after
that letting the mmc core figure out which module is actually soldered on.


believe that using opt-in with a compatible such as simple-sdio-powerup
is by far the safest thing to do, and as an added advantage we don't need
to worry about how to deal with the future complex power on cases at all,
we leave all the room in the world for various future scenarios. since as
soon as the simple-sdio-powerup compatible is not there the mmc core will
behave as it does today.


Please remember that the DT is *supposed* to be an ABI - systems are
supposed to be able to ship with a fixed DT and have it work with random
operating systems they have no knowledge of (and may not even exist at
the time the DT is being created).  This means we can't rely on removing
a compatible string later on.


I know that the DT is an ABI, and I'm not arguing for removing support for
the simple-sdio-powerup compatible from the kernel when a more complex
case arrives, nor am I arguing to remove it from the DT for existing working
boards. The idea behind the simple-sdio-powerup compatible is that it makes
the simple powerup behavior opt-in. So if a new board comes along which
requires something more complex, the people working on this can do what ever
they want / need without the simple powerup code getting in the way, as
long as they don't *add* the simple-sdio-powerup compatible to their *new*
DT file.

Basically the idea behind the simple-sdio-powerup compatible is to make
the worst case scenario for more complex boards to be the scenario which
we have today, i.e. no support for sdio powerup at all, rather then having
something in place which actually may get in the way, making things worse
then they are today.


Hi Hans,

I recalled a recent patchset from Russell King. He was working on i.MX6 
platform with brcmfmac device and ended reworking sdhci/mmc host 
controller code in a series of patches [1]. Patch 34 might be similar to 
what you are trying to accomplish.


Regards,
Arend
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: representing sdio devices oob interrupt, clks, etc. in device tree

2014-05-26 Thread Arend van Spriel


On 05/26/14 10:07, Hans de Goede wrote:

Hi,

On 05/26/2014 09:59 AM, Chen-Yu Tsai wrote:

On Mon, May 26, 2014 at 3:51 PM, Arend van Sprielar...@broadcom.com  wrote:

+ Russell


snip


Hi Hans,

I recalled a recent patchset from Russell King. He was working on i.MX6
platform with brcmfmac device and ended reworking sdhci/mmc host controller
code in a series of patches [1]. Patch 34 might be similar to what you are
trying to accomplish.


I believe that is a resend of Olof's patch I mentioned early in this
discussion. :)


Ok,

I meant to refer to this thread [1]. Indeed, the patch is from Olof.

Regards,
Arend

[1] http://thread.gmane.org/gmane.linux.documentation/22805


Ok, assuming that is the case, then it seems to me that we are all moving
somewhat in the same direction, which is good :)

What I would like to propose is to move forward with Olof's patch with
2 changes made to it:

1) Store the clocks / resets / whatever in childnodes of the mmc host node,
with the childnodes using the addressing scheme described in the patch
from Sascha Hauer titled: mmc: Add SDIO function devicetree subnode parsing,
as this is where they really belong (and in some cases the sdio function driver
may need access to them too).

2) Make Olof's code only do the powerup if the child node has a compatible of
simple-sdio-powerup, to avoid it getting in the way of more complex poweron
scenarios (which may require a separate pmic driver or some such) later.

Regards,

Hans


--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/11] brcmfmac: Fix OOB interrupt not working for BCM43362

2014-05-26 Thread Arend van Spriel


On 05/26/14 09:48, Hans de Goede wrote:

It has taken me a long long time to get the OOB interrupt working on the
AP6210 sdio wifi/bt module found on various Allwinner A20 boards. In the
end I found these magic register pokes in the cubietruck kernel tree:
https://github.com/cubieboard2/linux-sunxi/commit/7f08ba395617d17e7a711507503d89a50406fe7a

I'm not entirely sure if this specific to the AP6210 module, or if this
should be done for all BCM43362 sdio devices.


This magic is not in our host drivers so my guess is that it is board 
specific. This means the nvram file provided to the firmware should have 
this specified. Can you send me yours?


Regards,
Arend


Signed-off-by: Hans de Goedehdego...@redhat.com
---
  drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 18 ++
  1 file changed, 18 insertions(+)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c 
b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 0fc707c..2369a0f 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -39,7 +39,9 @@
  #includebrcm_hw_ids.h
  #includebrcmu_utils.h
  #includebrcmu_wifi.h
+#includechipcommon.h
  #includesoc.h
+#include chip.h
  #include dhd_bus.h
  #include dhd_dbg.h
  #include sdio_host.h
@@ -119,6 +121,7 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev 
*sdiodev)
  {
int ret = 0;
u8 data;
+   u32 addr, gpiocontrol;
unsigned long flags;

if ((sdiodev-pdata)  (sdiodev-pdata-oob_irq_supported)) {
@@ -148,6 +151,21 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev 
*sdiodev)

sdio_claim_host(sdiodev-func[1]);

+   if (sdiodev-bus_if-chip == BCM43362_CHIP_ID) {
+   addr = CORE_CC_REG(SI_ENUM_BASE, gpiocontrol);
+   gpiocontrol = brcmf_sdiod_regrl(sdiodev, addr,ret);
+   gpiocontrol |= 0x2;
+   brcmf_sdiod_regwl(sdiodev, addr, gpiocontrol,ret);
+
+   /* SPROM_ADDR_HIGH ? perhaps the defines name is off */
+   brcmf_sdiod_regwb(sdiodev, SBSDIO_SPROM_ADDR_HIGH, 0xf,
+   ret);
+   brcmf_sdiod_regwb(sdiodev, SBSDIO_CHIP_CTRL_DATA, 0,
+   ret);
+   brcmf_sdiod_regwb(sdiodev, SBSDIO_CHIP_CTRL_EN, 0x2,
+   ret);
+   }
+
/* must configure SDIO_CCCR_IENx to enable irq */
data = brcmf_sdiod_regrb(sdiodev, SDIO_CCCR_IENx,ret);
data |= 1  SDIO_FUNC_1 | 1  SDIO_FUNC_2 | 1;


--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: representing sdio devices oob interrupt, clks, etc. in device tree

2014-05-23 Thread Arend van Spriel


On 05/23/14 13:50, Hans de Goede wrote:

Hi,

On 05/23/2014 01:22 PM, Mark Brown wrote:

On Fri, May 23, 2014 at 11:13:44AM +0200, Hans de Goede wrote:


Thinking more about this, I would like to make one change to my
proposal, the mmc-core should only do power up of child-nodes if
they have a compatible of: simple-sdio-powerup. This way
when we add something more complex, we can keep the simple powerup
code in the mmc core, keeping what we've already using this working
and the mmc core won't respond to the child nodes for more complex
devices, so it won't conflict with more complex power-up handling
handled by some other driver.


Would it not be better to have this be something in the driver struct
rather than in the device tree?  Putting a compatible in there would be
encoding details of the Linux implementation in the DT which doesn't
seem right especially since these are details we're thinking of changing
later on.


The compatible is not a Linux specific thing, it is a marking saying
that something needs to take care of enabling the clks (and whatever
else we will make part of the binding for this compatible), before
scanning the mmc bus.


Something like have the driver set flags saying if it wants
to do complicated things.


Chicken-  egg, we won't know which driver to use before we've probed
the mmc bus, and we cannot probe the bus before enabling the clks, etc.


The approach I took with brcmfmac is that upon module init I search the 
DT for brcm,bcm43xx-fmac compatible and get the clock and/or gpio 
resource and enable them before registering the sdio driver. The 
difficulty is probably when using the driver built-in as the clocks and 
gpios may not be available yet and we can not rely on deferred probing 
in module init stage.


Regards,
Arend


FWIW if we ever get truely complex cases I think modeling the
power-up hardware as a pmic platform device is not a bad idea,
we would then need to have a generic mmc-host pmic property, which
would be used both to do the initial powerup before scanning, as
well as for the sdio device driver to get a handle to the pmic,
for run time power-management (if desired).


I don't know if this will ever apply to SDIO but with other buses the
complicated bits come when the driver wants to take over some of the
power management do things like turn some of the supplies or clocks on
and off independently at runtime for low power modes.


Hmm, good point in that case actually having these things in the
child node makes most sense, because then the driver can find them
their. Note that the mmc core enabling things does not mean that
the driver cannot later disable them if needed.


--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: representing sdio devices oob interrupt, clks, etc. in device tree

2014-05-23 Thread Arend van Spriel


On 05/23/14 15:28, Hans de Goede wrote:

Hi,

On 05/23/2014 03:21 PM, Arend van Spriel wrote:

On 05/23/14 13:50, Hans de Goede wrote:

Hi,

On 05/23/2014 01:22 PM, Mark Brown wrote:

On Fri, May 23, 2014 at 11:13:44AM +0200, Hans de Goede wrote:


Thinking more about this, I would like to make one change to my
proposal, the mmc-core should only do power up of child-nodes if
they have a compatible of: simple-sdio-powerup. This way
when we add something more complex, we can keep the simple powerup
code in the mmc core, keeping what we've already using this working
and the mmc core won't respond to the child nodes for more complex
devices, so it won't conflict with more complex power-up handling
handled by some other driver.


Would it not be better to have this be something in the driver struct
rather than in the device tree?  Putting a compatible in there would be
encoding details of the Linux implementation in the DT which doesn't
seem right especially since these are details we're thinking of changing
later on.


The compatible is not a Linux specific thing, it is a marking saying
that something needs to take care of enabling the clks (and whatever
else we will make part of the binding for this compatible), before
scanning the mmc bus.


Something like have the driver set flags saying if it wants
to do complicated things.


Chicken-   egg, we won't know which driver to use before we've probed
the mmc bus, and we cannot probe the bus before enabling the clks, etc.


The approach I took with brcmfmac is that upon module init I search the DT for 
brcm,bcm43xx-fmac compatible and get the clock and/or gpio resource and 
enable them before registering the sdio driver. The difficulty is probably when using the 
driver built-in as the clocks and gpios may not be available yet and we can not rely on 
deferred probing in module init stage.


I know, and that approach does not work, by the time the brcmfmac loads,
the mmc core has long probed the mmc bus and decided no one is home.


Ok. That is due to the non-removable property, right? I assumed a mmc 
rescan, which is (supposedly) triggered upon sdio driver registration, 
would subsequently find the device.


Regards,
Arend


Regards,

Hans



--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v13 2/2] ARM: sunxi: Add driver for SD/MMC hosts found on Allwinner sunxi SoCs

2014-05-12 Thread Arend van Spriel


On 05/12/2014 02:04 PM, Hans de Goede wrote:

From: David Lanzendörferdavid.lanzendoer...@o2s.ch

The Allwinner sunxi mmc host uses dma in bus-master mode using a built-in
designware idmac controller, which is identical to the one found in the mmc-dw
hosts. However the rest of the host is not identical to mmc-dw, it deals with
sending stop commands in hardware which makes it significantly different
from the mmc-dw devices.

HdG: Various cleanups and fixes.


Just nitpicking, but usually the above line should be added below the 
original signoff, so:



Signed-off-by: David Lanzendörferdavid.lanzendoer...@o2s.ch

[hdego...@redhat.com: various cleanups and fixes]

Signed-off-by: Hans de Goedehdego...@redhat.com


As is documented in SubmittingPatches.

Gr. AvS


---
  .../devicetree/bindings/mmc/sunxi-mmc.txt  |   43 +
  drivers/mmc/host/Kconfig   |7 +
  drivers/mmc/host/Makefile  |2 +
  drivers/mmc/host/sunxi-mmc.c   | 1049 
  4 files changed, 1101 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/mmc/sunxi-mmc.txt
  create mode 100644 drivers/mmc/host/sunxi-mmc.c

diff --git a/Documentation/devicetree/bindings/mmc/sunxi-mmc.txt 
b/Documentation/devicetree/bindings/mmc/sunxi-mmc.txt
new file mode 100644
index 000..91b3a34
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sunxi-mmc.txt
@@ -0,0 +1,43 @@
+* Allwinner sunxi MMC controller
+
+The highspeed MMC host controller on Allwinner SoCs provides an interface
+for MMC, SD and SDIO types of memory cards.
+
+Supported maximum speeds are the ones of the eMMC standard 4.5 as well
+as the speed of SD standard 3.0.
+Absolute maximum transfer rate is 200MB/s
+
+Required properties:
+ - compatible : allwinner,sun4i-a10-mmc or allwinner,sun5i-a13-mmc
+ - reg : mmc controller base registers
+ - clocks : a list with 2 phandle + clock specifier pairs
+ - clock-names : must contain ahb and mmc
+ - interrupts : mmc controller interrupt
+
+Optional properties:
+ - resets : phandle + reset specifier pair
+ - reset-names : must contain ahb
+ - for cd, bus-width and additional generic mmc parameters
+   please refer to mmc.txt within this directory
+
+Examples:
+   - Within .dtsi:
+   mmc0: mmc@01c0f000 {
+   compatible = allwinner,sun5i-a13-mmc;
+   reg =0x01c0f000 0x1000;
+   clocks =ahb_gates 8,mmc0_clk;
+   clock-names = ahb, mod;
+   interrupts =0 32 4;
+   status = disabled;
+   };
+
+   - Within dts:
+   mmc0: mmc@01c0f000 {
+   pinctrl-names = default, default;
+   pinctrl-0 =mmc0_pins_a;
+   pinctrl-1 =mmc0_cd_pin_reference_design;
+   bus-width =4;
+   cd-gpios =pio 7 1 0; /* PH1 */
+   cd-inverted;
+   status = okay;
+   };
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 8aaf8c1..d50ac1c 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -694,3 +694,10 @@ config MMC_REALTEK_PCI
help
  Say Y here to include driver code to support SD/MMC card interface
  of Realtek PCI-E card reader
+
+config MMC_SUNXI
+   tristate Allwinner sunxi SD/MMC Host Controller support
+   depends on ARCH_SUNXI
+   help
+ This selects support for the SD/MMC Host Controller on
+ Allwinner sunxi SoCs.
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 0c8aa5e..c706c0f 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -53,6 +53,8 @@ obj-$(CONFIG_MMC_WMT) += wmt-sdmmc.o

  obj-$(CONFIG_MMC_REALTEK_PCI) += rtsx_pci_sdmmc.o

+obj-$(CONFIG_MMC_SUNXI)+= sunxi-mmc.o
+
  obj-$(CONFIG_MMC_SDHCI_PLTFM) += sdhci-pltfm.o
  obj-$(CONFIG_MMC_SDHCI_CNS3XXX)   += sdhci-cns3xxx.o
  obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX) += sdhci-esdhc-imx.o
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
new file mode 100644
index 000..11bd48f
--- /dev/null
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -0,0 +1,1049 @@
+/*
+ * Driver for sunxi SD/MMC host controllers
+ * (C) Copyright 2007-2011 Reuuimlla Technology Co., Ltd.
+ * (C) Copyright 2007-2011 Aaron Maoyeleafy.m...@reuuimllatech.com
+ * (C) Copyright 2013-2014 O2S GmbHwww.o2s.ch
+ * (C) Copyright 2013-2014 David Lanzend�rferdavid.lanzendoer...@o2s.ch
+ * (C) Copyright 2013-2014 Hans de Goedehdego...@redhat.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+
+#includelinux/kernel.h
+#includelinux/module.h
+#includelinux/io.h
+#includelinux/device.h
+#includelinux/interrupt.h
+#includelinux/delay.h
+#includelinux/err.h
+
+#includelinux/clk.h

Re: [RFC] dt: bindings: add bindings for Broadcom bcm43xx sdio devices

2014-03-30 Thread Arend van Spriel


On 02/13/14 10:28, Chen-Yu Tsai wrote:

Hi,

On Thu, Feb 13, 2014 at 5:13 PM, Tomasz Figatomasz.f...@gmail.com  wrote:

Hi Arend,


On 10.02.2014 20:17, Arend van Spriel wrote:


[...]


+
+Example:
+
+mmc3: mmc@01c2 {
+   pinctrl-0 =mmc3_pins;
+   pinctrl-1 =wifi_host_wake;



WLAN_HOST_WAKE pin (aka the OOB interrupt) is specific to the WLAN chip, so
this should be rather configured in a pinctrl state of the WLAN chip itself.


Hi Chen-Yu,

picking up this thread.


AFAIK, the pinctrl in tied to the device node, and is selected when the device
is registered. The MMC subsystem currently does not register child nodes, so
this would be useless.


So if MMC does not register child nodes, brcmfmac will not be probed 
with of_node set? Have there been patches submitted for this in mmc 
subsystem recently.



brcmfmac actually has to walk the whole DT to find the node with the right
compatible.


Is it just me or should this be avoided? What if there are multiple entries?

Regards,
Arend
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] dt: bindings: add bindings for Broadcom bcm43xx sdio devices

2014-03-13 Thread Arend van Spriel

On 02/25/2014 11:51 PM, Stephen Warren wrote:
 On 02/10/2014 12:17 PM, Arend van Spriel wrote:
 The Broadcom bcm43xx sdio devices are fullmac devices that may be
 integrated in ARM platforms. Currently, the brcmfmac driver for
 these devices support use of platform data. This patch specifies
 the bindings that allow this platform data to be expressed in the
 devicetree.
 
 diff --git 
 a/Documentation/devicetree/bindings/staging/net/wireless/brcm,bcm43xx-fmac.txt
  
 b/Documentation/devicetree/bindings/staging/net/wireless/brcm,bcm43xx-fmac.txt
 
 + - compatible : Should be brcm,bcm43xx-fmac.
 + - wlan-supply : phandle for fixed regulator used to control power for
 +the device/module.
 
 Ignoring the fact that perhaps this should just be a GPIO instead and
 assuming it actually make sense for this to be a regulator:
 
 Why fixed regulator not just the regulator. There shouldn't be any
 requirement for the power supply to the device to be fixed; the driver
 should (a) set the voltage (which will be a no-op for a fixed regulator
 already providing that voltage), then (b) enable the regulator. That
 would allow a PMIC with programmable voltage to be feeding the device.
 
 Now, if this property was really intended to control some enable GPIO on
 the device, as others have said, this shouldn't be a regulator property
 but rather a GPIO property. However, there is definitely some power
 supply fed to the device, so you definitely need /some/ supply property
 here.
 
 Aren't there other enable GPIOs required? These should be specified in DT.
 
 Doesn't the WiFi chip/module require a (32KHz?) clock? If so, that needs
 to be represented in DT. Preferably write the binding to require
 clock-names (name-based lookup) rather than just clocks (index-based
 lookup).

Hi Stephen,

Thanks for these comments. While I agree with most of them, I am having
some difficulty with the DT concept. Essentially, a DT node describes a
part of the system. My scope for this change is probably limited wearing
my brcmfmac glasses. Am I correct in assuming that a DT node may be
processed/used by multiple drivers. As an example, the 32 kHz clock is
not something brcmfmac cares about. It simple needs to be available and
hooked up to the wlan device. The DT should have another node for this
clock which a (common) clock driver picks up. So having it referenced in
this node is purely informational, right?

Regards,
Arend

 +Optional properties:
 + - drive-strength : drive strength used for SDIO pins on device (default = 
 6mA).
 
 As mentioned elsewhere, since that's a binding-specific property, rename
 it brcm,drive-strength.
 
 + - interrupt-parent : the phandle for the interrupt controller to which the
 +device interrupt (HOST_WAKE) is connected.
 
 That's such a common property, individual bindings don't typically
 mention it.
 
 + - interrupts : interrupt specifier encoded according the interrupt 
 controller
 +specified by interrupt-parent property.
 
 The description of the property should say which interrupt (name and/or
 description) it's describing, even if there's only 1.
 
 
 +mmc3: mmc@01c2 {
 +pinctrl-0 = mmc3_pins;
 +pinctrl-1 = wifi_host_wake;
 +vmmc-supply = mmc3_supply;
 +bus-width = 4;
 
 None of that is really relevant to this binding, and may vary from SDIO
 controller to SDIO controller, so may end up being wrong.
 
 I'm not sure whether it makes sense to show the example inside some
 arbitrary SDIO controller node. Perhaps /just/ put the WiFi node in the
 example? The text above should be enough to describe that the node
 should be inside an SDIO controller.
 
 +bcm4335: bcm4335@0 {
 +compatible = brcm,bcm43xx-fmac;
 +wlan-supply = wlan-reg;
 +drive-strength = 4;
 +interrupt-parent = gpx2;
 +interrupts = 5 IRQ_TYPE_LEVEL_HIGH;
 +interrupt-names = HOST_WAKE;
 
 interrupt-names wasn't documented in the list of properties above.
 Entries in *-names properties are usually lower-case.
 

--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] dt: bindings: add bindings for Broadcom bcm43xx sdio devices

2014-03-13 Thread Arend van Spriel

On 03/13/2014 11:42 AM, Tomasz Figa wrote:
 Hi Arend,
 
 On 13.03.2014 11:16, Arend van Spriel wrote:
 On 02/25/2014 11:51 PM, Stephen Warren wrote:
 On 02/10/2014 12:17 PM, Arend van Spriel wrote:
 The Broadcom bcm43xx sdio devices are fullmac devices that may be
 integrated in ARM platforms. Currently, the brcmfmac driver for
 these devices support use of platform data. This patch specifies
 the bindings that allow this platform data to be expressed in the
 devicetree.

 diff --git
 a/Documentation/devicetree/bindings/staging/net/wireless/brcm,bcm43xx-fmac.txt
 b/Documentation/devicetree/bindings/staging/net/wireless/brcm,bcm43xx-fmac.txt


 + - compatible : Should be brcm,bcm43xx-fmac.
 + - wlan-supply : phandle for fixed regulator used to control power for
 +the device/module.

 Ignoring the fact that perhaps this should just be a GPIO instead and
 assuming it actually make sense for this to be a regulator:

 Why fixed regulator not just the regulator. There shouldn't be any
 requirement for the power supply to the device to be fixed; the driver
 should (a) set the voltage (which will be a no-op for a fixed regulator
 already providing that voltage), then (b) enable the regulator. That
 would allow a PMIC with programmable voltage to be feeding the device.

 Now, if this property was really intended to control some enable GPIO on
 the device, as others have said, this shouldn't be a regulator property
 but rather a GPIO property. However, there is definitely some power
 supply fed to the device, so you definitely need /some/ supply property
 here.

 Aren't there other enable GPIOs required? These should be specified
 in DT.

 Doesn't the WiFi chip/module require a (32KHz?) clock? If so, that needs
 to be represented in DT. Preferably write the binding to require
 clock-names (name-based lookup) rather than just clocks (index-based
 lookup).

 Hi Stephen,

 Thanks for these comments. While I agree with most of them, I am having
 some difficulty with the DT concept. Essentially, a DT node describes a
 part of the system.
 
 That's correct. A DT node represents a component of a system and its
 contents should contain all resources and other device-specific data
 required for this device to operate or optional.
 
 My scope for this change is probably limited wearing
 my brcmfmac glasses. Am I correct in assuming that a DT node may be
 processed/used by multiple drivers.
 
 It may be, but it is usually not. The typical use case for such scheme
 is a bus-like topology, where devices on the bus are sub-nodes of the
 bus controller node and may contain some bus-specific information, such
 as chip select (e.g. SPI), address (e.g. I2C) or maximum bus speed.
 
 As an example, the 32 kHz clock is
 not something brcmfmac cares about. It simple needs to be available and
 hooked up to the wlan device.
 
 Not really. The driver should care about any resources needed for the
 device to operate. In this case, a 32 kHz clock even if wired to the
 chip, sometimes is not operational until it gets ungated. This is not an
 artificial example, as on many boards I used to work with the 32 kHz
 clock was driven by a PMIC with clock gating control through I2C, gated
 by default.
 
 Moreover, (well, 32 kHz might not be the best example) from power saving
 reasons, it might be a good idea to let the driver control the clock and
 gate it whenever it is not necessary.

Hi Tomasz,

Thanks. That clarifies things.

 The DT should have another node for this
 clock which a (common) clock driver picks up. So having it referenced in
 this node is purely informational, right?
 
 You are confusing here provider with consumer. The bcm43xx chip is
 clearly a consumer of a 32 kHz clock and so its DT node should specify
 this.
 
 A DT node for a clock, would be a clock provider node and that would be
 handled by common clock framework in case of Linux indeed. A clock
 provider node doesn't have to be limited to a single clock, though. In
 the case I mentioned above, PMIC node would be a clock provider and PMIC
 driver would register necessary clocks in common clock framework.

I see. I figured the provider driver would not do that when the device
tree did not contain a consumer. Either, it is now clear what is
required from brcmfmac driver regarding clocks and gpios. Just still not
sure about the wlan-supply property. Does it depend on the specific
platform whether it is a gpio or regulator, ie. should I support both
(mutual exclusive or not).

Regards,
Arend
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: tegra: add device tree for SHIELD

2014-02-26 Thread Arend van Spriel

On 02/26/2014 05:52 AM, Alexandre Courbot wrote:
 On 02/25/2014 06:52 PM, Arend van Spriel wrote:
 On 02/25/2014 03:13 AM, Alexandre Courbot wrote:

 +/* Wifi */
 +sdhci@7800 {
 +status = okay;
 +bus-width = 4;
 +broken-cd;
 +keep-power-in-suspend;
 +cap-sdio-irq;

 Is non-removable better than broken-cd, or are they entirely unrelated?

 They are unrelated actually. With non-removable the driver expects the
 device to always be there since boot, and does not check for the card to
 be removed/added after boot. broken-cd indicates there is no CD line and
 the device should be polled regularly.

 For the Wifi chip, non-removable would be the correct setting
 hardware-wise, but there is a trap: the chip has its reset line asserted
 at boot-time, and you need to set GPIO 229 to de-assert it. Only after
 that will the device be detected on the SDIO bus. Since it lacks a CD
 line, it must be polled, hence the broken-cd property.

 This also raises another, redundant problem with DT bindings: AFAIK we
 currently have no way to let the system know the device will only appear
 after a given GPIO is set. It would also be nice to be able to give some
 parameters to the Wifi driver through the DT (like the OOB interrupt).
 Right now the Wifi chip is brought up by exporting the GPIO and writing
 to it from user-space, and the OOB interrupt is not used.

 Hi Alexandre,

 I recently posted a proposal for brcmfmac DT binding [1]. I did receive
 some comments, but it would be great if you (and/or others involved) had
 a look at it as well and give me some feedback. DT work still needs to
 grow on me.
 
 Hi Arend, (and thanks again for all the help with getting the chip to
 work!)
 
 Great, I'm not subscribed to the devicetree list and so have missed this
 thread, but I'm glad to see it.
 
 I don't think I have much to add to the comments you already received
 there. I'd need it to reference the 32K clock (which I currently
 force-enable manually), the OOB interrupt, and the reset pin as a GPIO
 (as for SHIELD the device needs to be put out of reset using an
 active-low GPIO before anything can happen). That last property could be
 optional as I suspect most designs won't use it.
 
 Getting the device out of reset should be done before the bus probes the
 non-removable device, so I wonder how this would fit wrt. the DT
 power-on sequencing series by Olof. Something tells me this could rather
 be a property of the bus, but physically speaking the pin is connected
 to the wifi chip, so... Maybe we could get the platform driver to ask
 the bus to probe again after enabling power/getting the device out of
 reset?

Actually, brcmfmac provides a platform driver and a sdio driver. At the
end of the platform probe it registers the sdio driver, which will
trigger the bus to probe again. I am not sure how that would relate to
the DT power-on sequencing you mentioned.

Regards,
Arend
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: tegra: add device tree for SHIELD

2014-02-25 Thread Arend van Spriel

On 02/25/2014 03:13 AM, Alexandre Courbot wrote:

 +/* Wifi */
 +sdhci@7800 {
 +status = okay;
 +bus-width = 4;
 +broken-cd;
 +keep-power-in-suspend;
 +cap-sdio-irq;

 Is non-removable better than broken-cd, or are they entirely unrelated?
 
 They are unrelated actually. With non-removable the driver expects the
 device to always be there since boot, and does not check for the card to
 be removed/added after boot. broken-cd indicates there is no CD line and
 the device should be polled regularly.
 
 For the Wifi chip, non-removable would be the correct setting
 hardware-wise, but there is a trap: the chip has its reset line asserted
 at boot-time, and you need to set GPIO 229 to de-assert it. Only after
 that will the device be detected on the SDIO bus. Since it lacks a CD
 line, it must be polled, hence the broken-cd property.
 
 This also raises another, redundant problem with DT bindings: AFAIK we
 currently have no way to let the system know the device will only appear
 after a given GPIO is set. It would also be nice to be able to give some
 parameters to the Wifi driver through the DT (like the OOB interrupt).
 Right now the Wifi chip is brought up by exporting the GPIO and writing
 to it from user-space, and the OOB interrupt is not used.

Hi Alexandre,

I recently posted a proposal for brcmfmac DT binding [1]. I did receive
some comments, but it would be great if you (and/or others involved) had
a look at it as well and give me some feedback. DT work still needs to
grow on me.

 Otherwise, Wifi works great with the brcmfmac driver and NVRAM file
 extracted from Android.

With in-band interrupts, indeed. The HOST_WAKE signal is the OOB
interrupt which would need to be provided in the device-tree.

Regards,
Arend

[1] http://mid.gmane.org/1392059868-8782-1-git-send-email-ar...@broadcom.com

--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] dt: bindings: add bindings for Broadcom bcm43xx sdio devices

2014-02-13 Thread Arend van Spriel

On 02/13/2014 10:13 AM, Tomasz Figa wrote:
 Hi Arend,
 
 On 10.02.2014 20:17, Arend van Spriel wrote:
 The Broadcom bcm43xx sdio devices are fullmac devices that may be
 integrated in ARM platforms. Currently, the brcmfmac driver for
 these devices support use of platform data. This patch specifies
 the bindings that allow this platform data to be expressed in the
 devicetree.

 Cc: Chen-Yu Tsai w...@csie.org
 Cc: Tomasz Figa tomasz.f...@gmail.com
 Reviewed-by: Hante Meuleman meule...@broadcom.com
 Reviewed-by: Pieter-Paul Giesberts piete...@broadcom.com
 Signed-off-by: Arend van Spriel ar...@broadcom.com
 ---
 This devicetree binding proposal is intended for platforms with
 Broadcom wireless device in MMC sdio slot. These devices may
 have their own interrupt and power line. Also the SDIO drive
 strength is often hardware dependent and expressed in this
 binding.

 Not sure if this should go in staging or not. Feel free to
 comment on this proposal.

 Regards,
 Arend
 ---
   .../staging/net/wireless/brcm,bcm43xx-fmac.txt |   37
 
   1 file changed, 37 insertions(+)
   create mode 100644
 Documentation/devicetree/bindings/staging/net/wireless/brcm,bcm43xx-fmac.txt


 diff --git
 a/Documentation/devicetree/bindings/staging/net/wireless/brcm,bcm43xx-fmac.txt
 b/Documentation/devicetree/bindings/staging/net/wireless/brcm,bcm43xx-fmac.txt

 new file mode 100644
 index 000..535f343
 --- /dev/null
 +++
 b/Documentation/devicetree/bindings/staging/net/wireless/brcm,bcm43xx-fmac.txt

 @@ -0,0 +1,37 @@
 +Broadcom BCM43xx Fullmac wireless SDIO devices
 +
 +This node provides properties for controlling the Broadcom wireless
 device. The
 +node is expected to be specified as a child node to the MMC
 controller that
 +connects the device to the system.
 +
 +Required properties:
 +
 + - compatible : Should be brcm,bcm43xx-fmac.
 + - wlan-supply : phandle for fixed regulator used to control power for
 +the device/module.
 
 The BCM43xx WLAN chips I used to work always have been controlled by a
 simple power enable GPIO of the chip itself. Has this changed in newer
 chips?
 
 If you need to simply toggle a GPIO to control the power, you don't need
 to use the regulator API at all, controlling the GPIO directly.

Not sure I understand. Do you really mean 'chip' or 'wifi module' here.
The chip needs to be powered and for that it is hooked up to a
host/module provided GPIO (at least that is my understanding).

 +
 +Optional properties:
 + - drive-strength : drive strength used for SDIO pins on device
 (default = 6mA).
 
 This should be a part of the MMC binding, I think. Probably also moved
 under MMC controller's node, since it's a board-specific property
 altering the parameters of the MMC controller, not the WLAN chip.

It is an electrical interfacing parameter between MMC controller and the
device. The specified drive-strength here is used to configure the PMU
on the chip so it is really related to the the chip.

 + - interrupt-parent : the phandle for the interrupt controller to
 which the
 +device interrupt (HOST_WAKE) is connected.
 + - interrupts : interrupt specifier encoded according the interrupt
 controller
 +specified by interrupt-parent property.
 
 I would also add a clock here, since the BCM43xx chips usually need a
 32k clock to operate (or at least the ones I used to work with did). It
 can be optional, as not all systems can control this clock.
 
 +
 +Example:
 +
 +mmc3: mmc@01c2 {
 +pinctrl-0 = mmc3_pins;
 +pinctrl-1 = wifi_host_wake;
 
 WLAN_HOST_WAKE pin (aka the OOB interrupt) is specific to the WLAN chip,
 so this should be rather configured in a pinctrl state of the WLAN chip
 itself.

You mean that pinctrl-1 should move inside the brcm,bcm43xx-fmac node,
right?

 +vmmc-supply = mmc3_supply;
 +bus-width = 4;
 +
 +bcm4335: bcm4335@0 {
 
 nit: Why @0, if there is no reg property under this node?

I am not fluent with devicetree specifications (yet) nor know all the
conventions.

 Best regards,
 Tomasz

Thanks,
Arend

 +compatible = brcm,bcm43xx-fmac;
 +wlan-supply = wlan-reg;
 +drive-strength = 4;
 +interrupt-parent = gpx2;
 +interrupts = 5 IRQ_TYPE_LEVEL_HIGH;
 +interrupt-names = HOST_WAKE;
 +};
 +};
 +


--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] dt: bindings: add bindings for Broadcom bcm43xx sdio devices

2014-02-10 Thread Arend van Spriel

The Broadcom bcm43xx sdio devices are fullmac devices that may be
integrated in ARM platforms. Currently, the brcmfmac driver for
these devices support use of platform data. This patch specifies
the bindings that allow this platform data to be expressed in the
devicetree.

Cc: Chen-Yu Tsai w...@csie.org
Cc: Tomasz Figa tomasz.f...@gmail.com
Reviewed-by: Hante Meuleman meule...@broadcom.com
Reviewed-by: Pieter-Paul Giesberts piete...@broadcom.com
Signed-off-by: Arend van Spriel ar...@broadcom.com
---
This devicetree binding proposal is intended for platforms with
Broadcom wireless device in MMC sdio slot. These devices may
have their own interrupt and power line. Also the SDIO drive
strength is often hardware dependent and expressed in this
binding.

Not sure if this should go in staging or not. Feel free to
comment on this proposal.

Regards,
Arend
---
 .../staging/net/wireless/brcm,bcm43xx-fmac.txt |   37 
 1 file changed, 37 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/staging/net/wireless/brcm,bcm43xx-fmac.txt

diff --git 
a/Documentation/devicetree/bindings/staging/net/wireless/brcm,bcm43xx-fmac.txt 
b/Documentation/devicetree/bindings/staging/net/wireless/brcm,bcm43xx-fmac.txt
new file mode 100644
index 000..535f343
--- /dev/null
+++ 
b/Documentation/devicetree/bindings/staging/net/wireless/brcm,bcm43xx-fmac.txt
@@ -0,0 +1,37 @@
+Broadcom BCM43xx Fullmac wireless SDIO devices
+
+This node provides properties for controlling the Broadcom wireless device. The
+node is expected to be specified as a child node to the MMC controller that
+connects the device to the system.
+
+Required properties:
+
+ - compatible : Should be brcm,bcm43xx-fmac.
+ - wlan-supply : phandle for fixed regulator used to control power for
+   the device/module.
+
+Optional properties:
+ - drive-strength : drive strength used for SDIO pins on device (default = 
6mA).
+ - interrupt-parent : the phandle for the interrupt controller to which the
+   device interrupt (HOST_WAKE) is connected.
+ - interrupts : interrupt specifier encoded according the interrupt controller
+   specified by interrupt-parent property.
+
+Example:
+
+mmc3: mmc@01c2 {
+   pinctrl-0 = mmc3_pins;
+   pinctrl-1 = wifi_host_wake;
+   vmmc-supply = mmc3_supply;
+   bus-width = 4;
+
+   bcm4335: bcm4335@0 {
+   compatible = brcm,bcm43xx-fmac;
+   wlan-supply = wlan-reg;
+   drive-strength = 4;
+   interrupt-parent = gpx2;
+   interrupts = 5 IRQ_TYPE_LEVEL_HIGH;
+   interrupt-names = HOST_WAKE;
+   };
+};
+
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-sunxi] Firmware for Bluetooth (and wifi)

2014-01-28 Thread Arend van Spriel

On 01/27/2014 11:12 AM, Tomasz Figa wrote:
 The brcmfmac driver that consumes these DT nodes will have a closer look
 at the device obtaining the chipid during the probe and determine if it
 can support it. So the compatible string indicates that the device needs
 a so-called fullmac wireless driver opposed to a mac80211 aka. softmac
 wireless driver.
 
 The compatible string should guarantee that the chip ID register holds a
 valid value, so just wifi-fullmac or brcmfmac sounds too generic to

I am not sure I understand this requirement. Is the DT node claimed
somehow after of_find_matching_node() and unavailable to other drivers.

 me. The string must specify the family of chips with this chip ID scheme
 in a reasonably precise way. brcm,bcm43xx-fmac maybe? I still see a
 risk of, say, BCM43999 showing up, which would be a completely different
 chip. while having the model matching the pattern.

If a completely different chip, ie. BCM43999, shows up in a board the
device tree should not use brcm,bcm43xx-fmac. That would be an error
in the dts file, right? All the devices listed in your bindings patch
are treated the same, ie. *compatible* on DT level and hence can have
the same compatible property.

In my opinion that is what the compatible property is about. It
identifies how a specific category of devices is accessed/configured. As
an example please see [1]. It shows one compatible string for a binding
that is used for different MPIC controllers.

Just to be clear, I like your suggestion to use brcm,bcm43xx-fmac, but
felt you did not so added my explanation/point of view.

Regards,
Arend

[1] Documentation/devicetree/bindings/powerpc/fsl/mpic.txt
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-sunxi] Firmware for Bluetooth (and wifi)

2014-01-27 Thread Arend van Spriel

On 01/27/2014 01:20 AM, Tomasz Figa wrote:
Hi Chen-Yu, Arend,

On 26.01.2014 22:39, Arend van Spriel wrote:
On 01/26/2014 05:58 PM, Chen-Yu Tsai wrote:
Hi,

On Mon, Jan 27, 2014 at 12:34 AM, Hans de Goede hdego...@redhat.com
wrote:
Hi,

On 01/24/2014 04:38 AM, Chen-Yu Tsai wrote:

snip

Quick update, I've just tested:
https://github.com/wens/linux/commits/wip/sunxi-next-wifi

About this, I would like to move WiFi power control to a regulator,
and controlled by sunxi-mci via vmmc-supply (not supported ATM)

Actually the sunxi-mci.c driver already has support for an optional
regulator called vmmc.

I like this idea, I've done a version of the dt patch using a regulator
instead of rfkill here:
https://github.com/jwrdegoede/linux-sunxi/commit/8d200113b573549cdcdc1b2d5a5a1cad15cfbe07

This works as advertised and IMHO is the better solution.

I have a version in another branch I haven't pushed. I had it using an
always-on regulator. I can adjust it to use vmmc.

BTW, I'd like to do a patch for sunxi-mci to use the DT parsing code
in mmc core.
We should re-use code if possible, wouldn't you agree?

About the oob interrupt stuff not working, AFAIK you should set a
pinctrl
function (not input, but a function like mmc is a function) on the
pin in
question
for it to work as external interrupt, I believe you're not doing
so in
your
dts.

The pinctrl driver seems to set the function when the interrupt is
enabled.
Unfortunately we don't have any documentation/examples on how to
use them.
I will look into that later.

Hmm, but you also have a pinctrl entry in the dts setting it up as
gpio-input,
maybe that conflicts ?

I made a version with pinctrl entry setup as irq, got an interrupt,
but then the whole thing hung. Looks like pinctrl irqchip was not
properly handling chained interrupts. I have done a simple fix, and I
hope to test it tomorrow. Then I'll do some more testing with different
configurations and hopefully write some documents.

Hi Chen-Yu,

I had a look at github tree from Hans with your DT support patch for
brcmfmac. I applied it to my 3.14 tree and modified it a little. I guess
the bindings are still experimental, but I would prefer you to use brcm
instead of broadcom in the 'compatible' entry as found in
Documentation/devicetree/bindings/vendor-prefixes.txt. There is an
exception to this rule in the bindings (in net/broadcom-bcm87xx.txt),
but I guess that train has left and it is tricky to change it now.
In the brcmfmac patch you also use multiple of_device ids. Could we make
it more generic, ie. compatible = brcm,brcmfmac or brcm,wifi-fullmac
or whatever...

brcmfmac and wifi-fullmac strings sound to generic for me. What happens
if an incompatible brcmfmac chip shows up? I'd rather use something like
bcm43xx to cover existing chip family, if all the bcm43xx chips are
compatible, or something more specific otherwise.

The brcmfmac driver that consumes these DT nodes will have a closer look
at the device obtaining the chipid during the probe and determine if it
can support it. So the compatible string indicates that the device needs
a so-called fullmac wireless driver opposed to a mac80211 aka. softmac
wireless driver.

By the way, you might find this thread interesting:

http://thread.gmane.org/gmane.linux.kernel.mmc/24728/focus=24783

In addition to the general idea of handling power control, I have also
posted a link to my patch adding DT support to brcmfmac driver.

You peaked my interest and I will catch up reading the thread.

Regards,
Arend

--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [linux-sunxi] Firmware for Bluetooth (and wifi)

2014-01-26 Thread Arend van Spriel

On 01/26/2014 05:58 PM, Chen-Yu Tsai wrote:
 Hi,
 
 On Mon, Jan 27, 2014 at 12:34 AM, Hans de Goede hdego...@redhat.com wrote:
 Hi,

 On 01/24/2014 04:38 AM, Chen-Yu Tsai wrote:

 snip


 Quick update, I've just tested:
 https://github.com/wens/linux/commits/wip/sunxi-next-wifi


 About this, I would like to move WiFi power control to a regulator,
 and controlled by sunxi-mci via vmmc-supply (not supported ATM)


 Actually the sunxi-mci.c driver already has support for an optional
 regulator called vmmc.

 I like this idea, I've done a version of the dt patch using a regulator
 instead of rfkill here:
 https://github.com/jwrdegoede/linux-sunxi/commit/8d200113b573549cdcdc1b2d5a5a1cad15cfbe07

 This works as advertised and IMHO is the better solution.
 
 I have a version in another branch I haven't pushed. I had it using an
 always-on regulator. I can adjust it to use vmmc.
 
 BTW, I'd like to do a patch for sunxi-mci to use the DT parsing code
 in mmc core.
 We should re-use code if possible, wouldn't you agree?
 
 About the oob interrupt stuff not working, AFAIK you should set a pinctrl
 function (not input, but a function like mmc is a function) on the pin in
 question
 for it to work as external interrupt, I believe you're not doing so in
 your
 dts.


 The pinctrl driver seems to set the function when the interrupt is
 enabled.
 Unfortunately we don't have any documentation/examples on how to use them.
 I will look into that later.


 Hmm, but you also have a pinctrl entry in the dts setting it up as
 gpio-input,
 maybe that conflicts ?
 
 I made a version with pinctrl entry setup as irq, got an interrupt,
 but then the whole thing hung. Looks like pinctrl irqchip was not
 properly handling chained interrupts. I have done a simple fix, and I
 hope to test it tomorrow. Then I'll do some more testing with different
 configurations and hopefully write some documents.

Hi Chen-Yu,

I had a look at github tree from Hans with your DT support patch for
brcmfmac. I applied it to my 3.14 tree and modified it a little. I guess
the bindings are still experimental, but I would prefer you to use brcm
instead of broadcom in the 'compatible' entry as found in
Documentation/devicetree/bindings/vendor-prefixes.txt. There is an
exception to this rule in the bindings (in net/broadcom-bcm87xx.txt),
but I guess that train has left and it is tricky to change it now.
In the brcmfmac patch you also use multiple of_device ids. Could we make
it more generic, ie. compatible = brcm,brcmfmac or brcm,wifi-fullmac
or whatever...

Regards,
Arend
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

44 matches

Mail list logo