diff mbox

[v6] mwifiex: parse device tree node for PCIe

Message ID 1477084869-15612-1-git-send-email-rajatja@google.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show

Commit Message

Rajat Jain Oct. 21, 2016, 9:21 p.m. UTC
From: Xinming Hu <huxm@marvell.com>

This patch derives device tree node from pcie bus layer framework, and
fixes a minor memory leak in mwifiex_pcie_probe() (in failure path).
Device tree bindings file has been renamed(marvell-sd8xxx.txt ->
marvell-8xxx.txt) to accommodate PCIe changes.

Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Rajat Jain <rajatja@google.com>
Reviewed-by: Brian Norris <briannorris@chromium.org>
---
v2: Included vendor and product IDs in compatible strings for PCIe
chipsets(Rob Herring)
v3: Patch is created using -M option so that it will only include diff of
original and renamed files(Rob Herring)
Resend v3: Resending the patch because I missed to include device tree mailing
while sending v3.
v4: Fix error handling, also move-on even if no device tree node is present.
v5: Update commit log to include memory leak, return -EINVAL instead of -1.
v6: Remove an unnecessary error print, fix typo in commit log

 .../{marvell-sd8xxx.txt => marvell-8xxx.txt}       |  8 +++--
 drivers/net/wireless/marvell/mwifiex/pcie.c        | 36 +++++++++++++++++++---
 drivers/net/wireless/marvell/mwifiex/sta_cmd.c     |  3 +-
 3 files changed, 39 insertions(+), 8 deletions(-)
 rename Documentation/devicetree/bindings/net/wireless/{marvell-sd8xxx.txt => marvell-8xxx.txt} (91%)

Comments

Brian Norris Oct. 26, 2016, 8:17 p.m. UTC | #1
Hi Rajat,

On Fri, Oct 21, 2016 at 02:21:09PM -0700, Rajat Jain wrote:
> From: Xinming Hu <huxm@marvell.com>
> 
> This patch derives device tree node from pcie bus layer framework, and
> fixes a minor memory leak in mwifiex_pcie_probe() (in failure path).
> Device tree bindings file has been renamed(marvell-sd8xxx.txt ->
> marvell-8xxx.txt) to accommodate PCIe changes.
> 
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> Signed-off-by: Rajat Jain <rajatja@google.com>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
> ---
> v2: Included vendor and product IDs in compatible strings for PCIe
> chipsets(Rob Herring)
> v3: Patch is created using -M option so that it will only include diff of
> original and renamed files(Rob Herring)
> Resend v3: Resending the patch because I missed to include device tree mailing
> while sending v3.
> v4: Fix error handling, also move-on even if no device tree node is present.
> v5: Update commit log to include memory leak, return -EINVAL instead of -1.

I've been working on reworking some bugfixes for this driver, and I
noticed we have some problems w.r.t. memory leaks, and the "memory leak"
fix is not actually a fix. See below.

> v6: Remove an unnecessary error print, fix typo in commit log
> 
>  .../{marvell-sd8xxx.txt => marvell-8xxx.txt}       |  8 +++--
>  drivers/net/wireless/marvell/mwifiex/pcie.c        | 36 +++++++++++++++++++---
>  drivers/net/wireless/marvell/mwifiex/sta_cmd.c     |  3 +-
>  3 files changed, 39 insertions(+), 8 deletions(-)
>  rename Documentation/devicetree/bindings/net/wireless/{marvell-sd8xxx.txt => marvell-8xxx.txt} (91%)
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> similarity index 91%
> rename from Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
> rename to Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> index c421aba..dfe5f8e 100644
> --- a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> @@ -1,8 +1,8 @@
> -Marvell 8897/8997 (sd8897/sd8997) SDIO devices
> +Marvell 8897/8997 (sd8897/sd8997/pcie8997) SDIO/PCIE devices
>  ------
>  
> -This node provides properties for controlling the marvell sdio wireless device.
> -The node is expected to be specified as a child node to the SDIO controller that
> +This node provides properties for controlling the marvell sdio/pcie wireless device.
> +The node is expected to be specified as a child node to the SDIO/PCIE controller that
>  connects the device to the system.
>  
>  Required properties:
> @@ -10,6 +10,8 @@ Required properties:
>    - compatible : should be one of the following:
>  	* "marvell,sd8897"
>  	* "marvell,sd8997"
> +	* "pci11ab,2b42"
> +	* "pci1b4b,2b42"
>  
>  Optional properties:
>  
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index 3c3c4f1..f7c84d3 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -37,6 +37,22 @@ static struct mwifiex_if_ops pcie_ops;
>  
>  static struct semaphore add_remove_card_sem;
>  
> +static const struct of_device_id mwifiex_pcie_of_match_table[] = {
> +	{ .compatible = "pci11ab,2b42" },
> +	{ .compatible = "pci1b4b,2b42" },
> +	{ }
> +};
> +
> +static int mwifiex_pcie_probe_of(struct device *dev)
> +{
> +	if (!of_match_node(mwifiex_pcie_of_match_table, dev->of_node)) {
> +		dev_err(dev, "required compatible string missing\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int
>  mwifiex_map_pci_memory(struct mwifiex_adapter *adapter, struct sk_buff *skb,
>  		       size_t size, int flags)
> @@ -178,6 +194,7 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
>  					const struct pci_device_id *ent)
>  {
>  	struct pcie_service_card *card;
> +	int ret;
>  
>  	pr_debug("info: vendor=0x%4.04X device=0x%4.04X rev=%d\n",
>  		 pdev->vendor, pdev->device, pdev->revision);
> @@ -199,13 +216,24 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
>  		card->pcie.can_ext_scan = data->can_ext_scan;
>  	}
>  
> -	if (mwifiex_add_card(card, &add_remove_card_sem, &pcie_ops,
> -			     MWIFIEX_PCIE)) {
> -		pr_err("%s failed\n", __func__);
> -		return -1;
> +	/* device tree node parsing and platform specific configuration*/
> +	if (pdev->dev.of_node) {
> +		ret = mwifiex_pcie_probe_of(&pdev->dev);
> +		if (ret)
> +			goto err_free;
>  	}
>  
> +	ret = mwifiex_add_card(card, &add_remove_card_sem, &pcie_ops,
> +			       MWIFIEX_PCIE);
> +	if (ret) {
> +		pr_err("%s failed\n", __func__);
> +		goto err_free;

For most error cases in mwifiex_add_card(), we call cleanup_if(), which
currently calls kfree(card). It's currently unbalanced, so there are
*some* cases that leak. But...

> +	}
>  	return 0;
> +
> +err_free:
> +	kfree(card);

That means that most of the time, this is actually a double-free. I'd
rather have the leak than the double-free :)

Anyway, I have a patch in the works (as part of some device
init/teardown bugfixes) that will convert the allocation to
devm_kzalloc() and drop the kfree()'ing. So that'll fix the
aforementioned bug.

In your next revision (sorry), please just drop this "leak" fix.

Regards,
Brian

> +	return ret;
>  }
>  
>  /*
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> index 2a162c3..c8dccf5 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> @@ -2218,7 +2218,8 @@ int mwifiex_sta_init_cmd(struct mwifiex_private *priv, u8 first_sta, bool init)
>  		 * The cal-data can be read from device tree and/or
>  		 * a configuration file and downloaded to firmware.
>  		 */
> -		if (priv->adapter->iface_type == MWIFIEX_SDIO &&
> +		if ((priv->adapter->iface_type == MWIFIEX_SDIO ||
> +		    priv->adapter->iface_type == MWIFIEX_PCIE) &&
>  		    adapter->dev->of_node) {
>  			adapter->dt_node = adapter->dev->of_node;
>  			if (of_property_read_u32(adapter->dt_node,
> -- 
> 2.8.0.rc3.226.g39d4020
>
Dmitry Torokhov Oct. 26, 2016, 8:46 p.m. UTC | #2
On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
> Hi Rajat,
> 
> On Fri, Oct 21, 2016 at 02:21:09PM -0700, Rajat Jain wrote:
> > From: Xinming Hu <huxm@marvell.com>
> > 
> > This patch derives device tree node from pcie bus layer framework, and
> > fixes a minor memory leak in mwifiex_pcie_probe() (in failure path).
> > Device tree bindings file has been renamed(marvell-sd8xxx.txt ->
> > marvell-8xxx.txt) to accommodate PCIe changes.
> > 
> > Signed-off-by: Xinming Hu <huxm@marvell.com>
> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > Signed-off-by: Rajat Jain <rajatja@google.com>
> > Reviewed-by: Brian Norris <briannorris@chromium.org>
> > ---
> > v2: Included vendor and product IDs in compatible strings for PCIe
> > chipsets(Rob Herring)
> > v3: Patch is created using -M option so that it will only include diff of
> > original and renamed files(Rob Herring)
> > Resend v3: Resending the patch because I missed to include device tree mailing
> > while sending v3.
> > v4: Fix error handling, also move-on even if no device tree node is present.
> > v5: Update commit log to include memory leak, return -EINVAL instead of -1.
> 
> I've been working on reworking some bugfixes for this driver, and I
> noticed we have some problems w.r.t. memory leaks, and the "memory leak"
> fix is not actually a fix. See below.

Sorry, I just saw this... Why do we need devicetree data for
discoverable bus (PCI)? How does the driver work on systems that do not
use DT? Why do we need them to behave differently?

Thanks.
Brian Norris Oct. 26, 2016, 8:56 p.m. UTC | #3
On Wed, Oct 26, 2016 at 01:51:48PM -0700, Rajat Jain wrote:
>    On Wed, Oct 26, 2016 at 1:46 PM, Dmitry Torokhov
>    <dmitry.torokhov@gmail.com> wrote:
>      On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
>      Sorry, I just saw this... Why do we need devicetree data for
>      discoverable bus (PCI)? How does the driver work on systems that do not
>      use DT? Why do we need them to behave differently?
> 
>    There are a couple of out-of-band GPIO pins from Marvell chip that can
>    serve as wake-up pins (wake up the CPU when asserted). The Marvell chip
>    has to be told which GPIO pin is to be used as the wake-up pin. The pin to
>    be used is system / platform dependent. (On some systems it could be
>    GPIO13, on others it could be GPIO14 etc depending on how the marvell chip
>    is wired up to the CPU).

There's also calibration data. See "marvell,caldata*" and
"marvell,wakeup-pin" properties. Currently only for SDIO, in
Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt, but
we're adding support for PCIe.

Brian
Dmitry Torokhov Oct. 26, 2016, 9:06 p.m. UTC | #4
On Wed, Oct 26, 2016 at 01:56:34PM -0700, Brian Norris wrote:
> On Wed, Oct 26, 2016 at 01:51:48PM -0700, Rajat Jain wrote:
> >    On Wed, Oct 26, 2016 at 1:46 PM, Dmitry Torokhov
> >    <dmitry.torokhov@gmail.com> wrote:
> >      On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
> >      Sorry, I just saw this... Why do we need devicetree data for
> >      discoverable bus (PCI)? How does the driver work on systems that do not
> >      use DT? Why do we need them to behave differently?
> > 
> >    There are a couple of out-of-band GPIO pins from Marvell chip that can
> >    serve as wake-up pins (wake up the CPU when asserted). The Marvell chip
> >    has to be told which GPIO pin is to be used as the wake-up pin. The pin to
> >    be used is system / platform dependent. (On some systems it could be
> >    GPIO13, on others it could be GPIO14 etc depending on how the marvell chip
> >    is wired up to the CPU).

So wakeup pin is not wired to PCIe WAKE?

> There's also calibration data. See "marvell,caldata*" and
> "marvell,wakeup-pin" properties. Currently only for SDIO, in
> Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt, but
> we're adding support for PCIe.

How would it all work if I moved the PCIe module from one device to
another?

Thanks.
Brian Norris Oct. 26, 2016, 9:08 p.m. UTC | #5
On Wed, Oct 26, 2016 at 02:06:48PM -0700, Dmitry Torokhov wrote:
> On Wed, Oct 26, 2016 at 01:56:34PM -0700, Brian Norris wrote:
> > On Wed, Oct 26, 2016 at 01:51:48PM -0700, Rajat Jain wrote:
> > >    On Wed, Oct 26, 2016 at 1:46 PM, Dmitry Torokhov
> > >    <dmitry.torokhov@gmail.com> wrote:
> > >      On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
> > >      Sorry, I just saw this... Why do we need devicetree data for
> > >      discoverable bus (PCI)? How does the driver work on systems that do not
> > >      use DT? Why do we need them to behave differently?
> > > 
> > >    There are a couple of out-of-band GPIO pins from Marvell chip that can
> > >    serve as wake-up pins (wake up the CPU when asserted). The Marvell chip
> > >    has to be told which GPIO pin is to be used as the wake-up pin. The pin to
> > >    be used is system / platform dependent. (On some systems it could be
> > >    GPIO13, on others it could be GPIO14 etc depending on how the marvell chip
> > >    is wired up to the CPU).
> 
> So wakeup pin is not wired to PCIe WAKE?

Not in our case.

> > There's also calibration data. See "marvell,caldata*" and
> > "marvell,wakeup-pin" properties. Currently only for SDIO, in
> > Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt, but
> > we're adding support for PCIe.
> 
> How would it all work if I moved the PCIe module from one device to
> another?

These boards are soldered down, at least in the case I care about.

Brian
Rajat Jain Oct. 26, 2016, 9:16 p.m. UTC | #6
On Wed, Oct 26, 2016 at 2:08 PM, Brian Norris <briannorris@chromium.org> wrote:
> On Wed, Oct 26, 2016 at 02:06:48PM -0700, Dmitry Torokhov wrote:
>> On Wed, Oct 26, 2016 at 01:56:34PM -0700, Brian Norris wrote:
>> > On Wed, Oct 26, 2016 at 01:51:48PM -0700, Rajat Jain wrote:
>> > >    On Wed, Oct 26, 2016 at 1:46 PM, Dmitry Torokhov
>> > >    <dmitry.torokhov@gmail.com> wrote:
>> > >      On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
>> > >      Sorry, I just saw this... Why do we need devicetree data for
>> > >      discoverable bus (PCI)? How does the driver work on systems that do not
>> > >      use DT? Why do we need them to behave differently?
>> > >
>> > >    There are a couple of out-of-band GPIO pins from Marvell chip that can
>> > >    serve as wake-up pins (wake up the CPU when asserted). The Marvell chip
>> > >    has to be told which GPIO pin is to be used as the wake-up pin. The pin to
>> > >    be used is system / platform dependent. (On some systems it could be
>> > >    GPIO13, on others it could be GPIO14 etc depending on how the marvell chip
>> > >    is wired up to the CPU).
>>
>> So wakeup pin is not wired to PCIe WAKE?
>
> Not in our case.
>
>> > There's also calibration data. See "marvell,caldata*" and
>> > "marvell,wakeup-pin" properties. Currently only for SDIO, in
>> > Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt, but
>> > we're adding support for PCIe.
>>
>> How would it all work if I moved the PCIe module from one device to
>> another?
>
> These boards are soldered down, at least in the case I care about.

That is right. Since the out of band wake-up pin is not standard on
the PCIe connector - this feature is for soldered chips only.


>
> Brian
Rob Herring (Arm) Oct. 30, 2016, 8:41 p.m. UTC | #7
On Fri, Oct 21, 2016 at 02:21:09PM -0700, Rajat Jain wrote:
> From: Xinming Hu <huxm@marvell.com>
> 
> This patch derives device tree node from pcie bus layer framework, and
> fixes a minor memory leak in mwifiex_pcie_probe() (in failure path).
> Device tree bindings file has been renamed(marvell-sd8xxx.txt ->
> marvell-8xxx.txt) to accommodate PCIe changes.
> 
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> Signed-off-by: Rajat Jain <rajatja@google.com>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
> ---
> v2: Included vendor and product IDs in compatible strings for PCIe
> chipsets(Rob Herring)
> v3: Patch is created using -M option so that it will only include diff of
> original and renamed files(Rob Herring)
> Resend v3: Resending the patch because I missed to include device tree mailing
> while sending v3.
> v4: Fix error handling, also move-on even if no device tree node is present.
> v5: Update commit log to include memory leak, return -EINVAL instead of -1.
> v6: Remove an unnecessary error print, fix typo in commit log
> 
>  .../{marvell-sd8xxx.txt => marvell-8xxx.txt}       |  8 +++--
>  drivers/net/wireless/marvell/mwifiex/pcie.c        | 36 +++++++++++++++++++---
>  drivers/net/wireless/marvell/mwifiex/sta_cmd.c     |  3 +-
>  3 files changed, 39 insertions(+), 8 deletions(-)
>  rename Documentation/devicetree/bindings/net/wireless/{marvell-sd8xxx.txt => marvell-8xxx.txt} (91%)
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> similarity index 91%
> rename from Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
> rename to Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> index c421aba..dfe5f8e 100644
> --- a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> @@ -1,8 +1,8 @@
> -Marvell 8897/8997 (sd8897/sd8997) SDIO devices
> +Marvell 8897/8997 (sd8897/sd8997/pcie8997) SDIO/PCIE devices
>  ------
>  
> -This node provides properties for controlling the marvell sdio wireless device.
> -The node is expected to be specified as a child node to the SDIO controller that
> +This node provides properties for controlling the marvell sdio/pcie wireless device.

s/marvell/Marvell/
s/sdio\/pcie/SDIO\/PCIE/

> +The node is expected to be specified as a child node to the SDIO/PCIE controller that
>  connects the device to the system.
>  
>  Required properties:
> @@ -10,6 +10,8 @@ Required properties:
>    - compatible : should be one of the following:
>  	* "marvell,sd8897"
>  	* "marvell,sd8997"
> +	* "pci11ab,2b42"
> +	* "pci1b4b,2b42"

I think I already said this, but you have the vendor and product IDs 
reversed.

Rob
Rajat Jain Oct. 31, 2016, 5:09 p.m. UTC | #8
On Sun, Oct 30, 2016 at 1:41 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Oct 21, 2016 at 02:21:09PM -0700, Rajat Jain wrote:
>> From: Xinming Hu <huxm@marvell.com>
>>
>> This patch derives device tree node from pcie bus layer framework, and
>> fixes a minor memory leak in mwifiex_pcie_probe() (in failure path).
>> Device tree bindings file has been renamed(marvell-sd8xxx.txt ->
>> marvell-8xxx.txt) to accommodate PCIe changes.
>>
>> Signed-off-by: Xinming Hu <huxm@marvell.com>
>> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
>> Signed-off-by: Rajat Jain <rajatja@google.com>
>> Reviewed-by: Brian Norris <briannorris@chromium.org>
>> ---
>> v2: Included vendor and product IDs in compatible strings for PCIe
>> chipsets(Rob Herring)
>> v3: Patch is created using -M option so that it will only include diff of
>> original and renamed files(Rob Herring)
>> Resend v3: Resending the patch because I missed to include device tree mailing
>> while sending v3.
>> v4: Fix error handling, also move-on even if no device tree node is present.
>> v5: Update commit log to include memory leak, return -EINVAL instead of -1.
>> v6: Remove an unnecessary error print, fix typo in commit log
>>
>>  .../{marvell-sd8xxx.txt => marvell-8xxx.txt}       |  8 +++--
>>  drivers/net/wireless/marvell/mwifiex/pcie.c        | 36 +++++++++++++++++++---
>>  drivers/net/wireless/marvell/mwifiex/sta_cmd.c     |  3 +-
>>  3 files changed, 39 insertions(+), 8 deletions(-)
>>  rename Documentation/devicetree/bindings/net/wireless/{marvell-sd8xxx.txt => marvell-8xxx.txt} (91%)
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
>> similarity index 91%
>> rename from Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
>> rename to Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
>> index c421aba..dfe5f8e 100644
>> --- a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
>> +++ b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
>> @@ -1,8 +1,8 @@
>> -Marvell 8897/8997 (sd8897/sd8997) SDIO devices
>> +Marvell 8897/8997 (sd8897/sd8997/pcie8997) SDIO/PCIE devices
>>  ------
>>
>> -This node provides properties for controlling the marvell sdio wireless device.
>> -The node is expected to be specified as a child node to the SDIO controller that
>> +This node provides properties for controlling the marvell sdio/pcie wireless device.
>
> s/marvell/Marvell/
> s/sdio\/pcie/SDIO\/PCIE/
>
>> +The node is expected to be specified as a child node to the SDIO/PCIE controller that
>>  connects the device to the system.
>>
>>  Required properties:
>> @@ -10,6 +10,8 @@ Required properties:
>>    - compatible : should be one of the following:
>>       * "marvell,sd8897"
>>       * "marvell,sd8997"
>> +     * "pci11ab,2b42"
>> +     * "pci1b4b,2b42"
>

Hi Rob,

> I think I already said this, but you have the vendor and product IDs
> reversed.

Actually Marvell has 2 vendor IDs assigned to it. In include/linux/pci_ids.h:

#define PCI_VENDOR_ID_MARVELL           0x11ab
#define PCI_VENDOR_ID_MARVELL_EXT       0x1b4b

So in this case the compatible property describes a single product ID,
with both possible vendor IDs.

Thanks,

Rajat



>
> Rob
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
similarity index 91%
rename from Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
rename to Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
index c421aba..dfe5f8e 100644
--- a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
+++ b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
@@ -1,8 +1,8 @@ 
-Marvell 8897/8997 (sd8897/sd8997) SDIO devices
+Marvell 8897/8997 (sd8897/sd8997/pcie8997) SDIO/PCIE devices
 ------
 
-This node provides properties for controlling the marvell sdio wireless device.
-The node is expected to be specified as a child node to the SDIO controller that
+This node provides properties for controlling the marvell sdio/pcie wireless device.
+The node is expected to be specified as a child node to the SDIO/PCIE controller that
 connects the device to the system.
 
 Required properties:
@@ -10,6 +10,8 @@  Required properties:
   - compatible : should be one of the following:
 	* "marvell,sd8897"
 	* "marvell,sd8997"
+	* "pci11ab,2b42"
+	* "pci1b4b,2b42"
 
 Optional properties:
 
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 3c3c4f1..f7c84d3 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -37,6 +37,22 @@  static struct mwifiex_if_ops pcie_ops;
 
 static struct semaphore add_remove_card_sem;
 
+static const struct of_device_id mwifiex_pcie_of_match_table[] = {
+	{ .compatible = "pci11ab,2b42" },
+	{ .compatible = "pci1b4b,2b42" },
+	{ }
+};
+
+static int mwifiex_pcie_probe_of(struct device *dev)
+{
+	if (!of_match_node(mwifiex_pcie_of_match_table, dev->of_node)) {
+		dev_err(dev, "required compatible string missing\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int
 mwifiex_map_pci_memory(struct mwifiex_adapter *adapter, struct sk_buff *skb,
 		       size_t size, int flags)
@@ -178,6 +194,7 @@  static int mwifiex_pcie_probe(struct pci_dev *pdev,
 					const struct pci_device_id *ent)
 {
 	struct pcie_service_card *card;
+	int ret;
 
 	pr_debug("info: vendor=0x%4.04X device=0x%4.04X rev=%d\n",
 		 pdev->vendor, pdev->device, pdev->revision);
@@ -199,13 +216,24 @@  static int mwifiex_pcie_probe(struct pci_dev *pdev,
 		card->pcie.can_ext_scan = data->can_ext_scan;
 	}
 
-	if (mwifiex_add_card(card, &add_remove_card_sem, &pcie_ops,
-			     MWIFIEX_PCIE)) {
-		pr_err("%s failed\n", __func__);
-		return -1;
+	/* device tree node parsing and platform specific configuration*/
+	if (pdev->dev.of_node) {
+		ret = mwifiex_pcie_probe_of(&pdev->dev);
+		if (ret)
+			goto err_free;
 	}
 
+	ret = mwifiex_add_card(card, &add_remove_card_sem, &pcie_ops,
+			       MWIFIEX_PCIE);
+	if (ret) {
+		pr_err("%s failed\n", __func__);
+		goto err_free;
+	}
 	return 0;
+
+err_free:
+	kfree(card);
+	return ret;
 }
 
 /*
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
index 2a162c3..c8dccf5 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
@@ -2218,7 +2218,8 @@  int mwifiex_sta_init_cmd(struct mwifiex_private *priv, u8 first_sta, bool init)
 		 * The cal-data can be read from device tree and/or
 		 * a configuration file and downloaded to firmware.
 		 */
-		if (priv->adapter->iface_type == MWIFIEX_SDIO &&
+		if ((priv->adapter->iface_type == MWIFIEX_SDIO ||
+		    priv->adapter->iface_type == MWIFIEX_PCIE) &&
 		    adapter->dev->of_node) {
 			adapter->dt_node = adapter->dev->of_node;
 			if (of_property_read_u32(adapter->dt_node,