This patch series adds native support for wl1271 on ZOOM.
Changes since v3:
- slot is marked nonremovable
- no explicit slot ocr mask
- dont split hsmmc mmc23 power control
Patches are based on linux-next (as of Aug 7th), and tested
on ZOOM2.
Please note that I am going to have a very limited email access
in the next three weeks, so I might not respond right away.
Thanks,
Ohad Ben-Cohen (8):
wireless: wl1271: make wl12xx.h common to both spi and sdio
wireless: wl1271: support return value for the set power func
wireless: wl1271: add platform driver to get board data
wireless: wl1271: take irq info from private board data
wireless: wl1271: make ref_clock configurable by board
omap: hsmmc: remove unused variable
omap: zoom: add fixed regulator device for wlan
omap: zoom: add mmc3/wl1271 device support
arch/arm/mach-omap2/board-zoom-peripherals.c | 69 +++++++++++++++
drivers/mmc/host/omap_hsmmc.c | 1 -
drivers/net/wireless/wl12xx/wl1251_sdio.c | 2 +-
drivers/net/wireless/wl12xx/wl1251_spi.c | 2 +-
drivers/net/wireless/wl12xx/wl1271.h | 15 +++-
drivers/net/wireless/wl12xx/wl1271_boot.c | 9 +-
drivers/net/wireless/wl12xx/wl1271_boot.h | 1 -
drivers/net/wireless/wl12xx/wl1271_io.h | 8 +-
drivers/net/wireless/wl12xx/wl1271_main.c | 4 +-
drivers/net/wireless/wl12xx/wl1271_sdio.c | 120 +++++++++++++++++++++++---
drivers/net/wireless/wl12xx/wl1271_spi.c | 8 ++-
include/linux/spi/wl12xx.h | 34 -------
include/linux/wl12xx.h | 35 ++++++++
13 files changed, 245 insertions(+), 63 deletions(-)
delete mode 100644 include/linux/spi/wl12xx.h
create mode 100644 include/linux/wl12xx.h
@@ -1,34 +0,0 @@-/*- * This file is part of wl12xx- *- * Copyright (C) 2009 Nokia Corporation- *- * Contact: Kalle Valo <kalle.valo@nokia.com>- *- * This program is free software; you can redistribute it and/or- * modify it under the terms of the GNU General Public License- * version 2 as published by the Free Software Foundation.- *- * This program is distributed in the hope that it will be useful, but- * WITHOUT ANY WARRANTY; without even the implied warranty of- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU- * General Public License for more details.- *- * You should have received a copy of the GNU General Public License- * along with this program; if not, write to the Free Software- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA- * 02110-1301 USA- *- */--#ifndef _LINUX_SPI_WL12XX_H-#define _LINUX_SPI_WL12XX_H--struct wl12xx_platform_data {- void (*set_power)(bool enable);- /* SDIO only: IRQ number if WLAN_IRQ line is used, 0 for SDIO IRQs */- int irq;- bool use_eeprom;-};--#endif
@@ -0,0 +1,34 @@+/*+*Thisfileispartofwl12xx+*+*Copyright(C)2009NokiaCorporation+*+*Contact:KalleValo<kalle.valo@nokia.com>+*+*Thisprogramisfreesoftware;youcanredistributeitand/or+*modifyitunderthetermsoftheGNUGeneralPublicLicense+*version2aspublishedbytheFreeSoftwareFoundation.+*+*Thisprogramisdistributedinthehopethatitwillbeuseful,but+*WITHOUTANYWARRANTY;withouteventheimpliedwarrantyof+*MERCHANTABILITYorFITNESSFORAPARTICULARPURPOSE.SeetheGNU+*GeneralPublicLicenseformoredetails.+*+*YoushouldhavereceivedacopyoftheGNUGeneralPublicLicense+*alongwiththisprogram;ifnot,writetotheFreeSoftware+*Foundation,Inc.,51FranklinSt,FifthFloor,Boston,MA+*02110-1301USA+*+*/++#ifndef _LINUX_WL12XX_H+#define _LINUX_WL12XX_H++structwl12xx_platform_data{+void(*set_power)(boolenable);+/* SDIO only: IRQ number if WLAN_IRQ line is used, 0 for SDIO IRQs */+intirq;+booluse_eeprom;+};++#endif
Make it possible for the set power method to indicate a
success/failure return value. This is needed to support
more complex power on/off operations such as bringing up
(and down) sdio functions.
Signed-off-by: Ohad Ben-Cohen <redacted>
---
drivers/net/wireless/wl12xx/wl1271.h | 2 +-
drivers/net/wireless/wl12xx/wl1271_io.h | 8 +++++---
drivers/net/wireless/wl12xx/wl1271_main.c | 4 +++-
drivers/net/wireless/wl12xx/wl1271_sdio.c | 4 +++-
drivers/net/wireless/wl12xx/wl1271_spi.c | 4 +++-
5 files changed, 15 insertions(+), 7 deletions(-)
Dynamically create and register a platform driver, that will
be used to to receive board-specific information like irq and
reference clock numbers.
The driver is created dynamically in order to avoid the 1-device
limitation of a fixed platform driver name.
Signed-off-by: Ohad Ben-Cohen <redacted>
---
drivers/net/wireless/wl12xx/wl1271.h | 12 ++++
drivers/net/wireless/wl12xx/wl1271_sdio.c | 103 +++++++++++++++++++++++++++--
2 files changed, 108 insertions(+), 7 deletions(-)
@@ -205,17 +283,24 @@ static int __devinit wl1271_probe(struct sdio_func *func,/* Grab access to FN0 for ELP reg. */func->card->quirks|=MMC_QUIRK_LENIENT_FN0;+wlan_data=wl1271_get_data(wl,func->card->host->index);+if(IS_ERR(wlan_data)){+ret=PTR_ERR(wlan_data);+wl1271_error("missing wlan data (needed for irq/ref_clk)!");+gotoout_free;+}+wl->irq=gpio_to_irq(RX71_WL1271_IRQ_GPIO);if(wl->irq<0){ret=wl->irq;wl1271_error("could not get irq!");-gotoout_free;+gotoput_data;}ret=request_irq(wl->irq,wl1271_irq,0,DRIVER_NAME,wl);if(ret<0){wl1271_error("request_irq() failed: %d",ret);-gotoout_free;+gotoput_data;}set_irq_type(wl->irq,IRQ_TYPE_EDGE_RISING);
@@ -236,13 +321,13 @@ static int __devinit wl1271_probe(struct sdio_func *func,return0;-out_irq:+out_irq:free_irq(wl->irq,wl);---out_free:+put_data:+platform_driver_unregister(&wl->pinstance->pdriver);+kfree(wl->pinstance);+out_free:wl1271_free_hw(wl);-returnret;}
Remove the hard coded irq information, and instead take
the irq information from the board's platform data.
Signed-off-by: Ohad Ben-Cohen <redacted>
---
drivers/net/wireless/wl12xx/wl1271_sdio.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
@@ -290,12 +295,7 @@ static int __devinit wl1271_probe(struct sdio_func *func,gotoout_free;}-wl->irq=gpio_to_irq(RX71_WL1271_IRQ_GPIO);-if(wl->irq<0){-ret=wl->irq;-wl1271_error("could not get irq!");-gotoput_data;-}+wl->irq=wlan_data->irq;ret=request_irq(wl->irq,wl1271_irq,0,DRIVER_NAME,wl);if(ret<0){
The wl1271 device is using a reference clock that may change
between board to board.
Make the ref_clock parameter configurable by the board
files that set up the device, instead of having a hard coded
value in the driver source itself.
Signed-off-by: Ohad Ben-Cohen <redacted>
---
drivers/net/wireless/wl12xx/wl1271.h | 1 +
drivers/net/wireless/wl12xx/wl1271_boot.c | 9 +++++----
drivers/net/wireless/wl12xx/wl1271_boot.h | 1 -
drivers/net/wireless/wl12xx/wl1271_sdio.c | 1 +
drivers/net/wireless/wl12xx/wl1271_spi.c | 2 ++
include/linux/wl12xx.h | 1 +
6 files changed, 10 insertions(+), 5 deletions(-)
@@ -29,6 +29,7 @@ struct wl12xx_platform_data {/* SDIO only: IRQ number if WLAN_IRQ line is used, 0 for SDIO IRQs */intirq;booluse_eeprom;+intboard_ref_clock;};#endif
Add MMC3 support on ZOOM, which has the wl1271 device hardwired to.
The wl1271 is a 4-wire, 1.8V, embedded SDIO WLAN device with an
external IRQ line, and power-controlled by a GPIO-based fixed regulator.
Signed-off-by: Ohad Ben-Cohen <redacted>
---
arch/arm/mach-omap2/board-zoom-peripherals.c | 34 ++++++++++++++++++++++++++
1 files changed, 34 insertions(+), 0 deletions(-)
From: DebBarma, Tarun Kanti <hidden> Date: 2010-08-11 18:35:04
Ohad,
quoted hunk
-----Original Message-----
From: linux-omap-owner at vger.kernel.org [mailto:linux-omap-
owner at vger.kernel.org] On Behalf Of Ohad Ben-Cohen
Sent: Wednesday, August 11, 2010 11:52 PM
To: linux-wireless at vger.kernel.org; linux-mmc at vger.kernel.org; linux-
omap at vger.kernel.org
Cc: Ido Yariv; Mark Brown; linux-arm-kernel at lists.infradead.org;
Chikkature Rajashekar, Madhusudhan; Luciano Coelho; akpm at linux-
foundation.org; San Mehat; Roger Quadros; Tony Lindgren; Nicolas Pitre;
Pandita, Vikram; Kalle Valo; Ohad Ben-Cohen
Subject: [PATCH v4 2/8] wireless: wl1271: support return value for the set
power func
Make it possible for the set power method to indicate a
success/failure return value. This is needed to support
more complex power on/off operations such as bringing up
(and down) sdio functions.
Signed-off-by: Ohad Ben-Cohen <redacted>
---
drivers/net/wireless/wl12xx/wl1271.h | 2 +-
drivers/net/wireless/wl12xx/wl1271_io.h | 8 +++++---
drivers/net/wireless/wl12xx/wl1271_main.c | 4 +++-
drivers/net/wireless/wl12xx/wl1271_sdio.c | 4 +++-
drivers/net/wireless/wl12xx/wl1271_spi.c | 4 +++-
5 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/wl1271.h
b/drivers/net/wireless/wl12xx/wl1271.h
index dd3cee6..faa5925 100644
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: DebBarma, Tarun Kanti <hidden> Date: 2010-08-11 18:42:18
Ohad,
quoted hunk
-----Original Message-----
From: linux-omap-owner at vger.kernel.org [mailto:linux-omap-
owner at vger.kernel.org] On Behalf Of Ohad Ben-Cohen
Sent: Wednesday, August 11, 2010 11:52 PM
To: linux-wireless at vger.kernel.org; linux-mmc at vger.kernel.org; linux-
omap at vger.kernel.org
Cc: Ido Yariv; Mark Brown; linux-arm-kernel at lists.infradead.org;
Chikkature Rajashekar, Madhusudhan; Luciano Coelho; akpm at linux-
foundation.org; San Mehat; Roger Quadros; Tony Lindgren; Nicolas Pitre;
Pandita, Vikram; Kalle Valo; Ohad Ben-Cohen
Subject: [PATCH v4 3/8] wireless: wl1271: add platform driver to get board
data
Dynamically create and register a platform driver, that will
be used to to receive board-specific information like irq and
reference clock numbers.
The driver is created dynamically in order to avoid the 1-device
limitation of a fixed platform driver name.
Signed-off-by: Ohad Ben-Cohen <redacted>
---
drivers/net/wireless/wl12xx/wl1271.h | 12 ++++
drivers/net/wireless/wl12xx/wl1271_sdio.c | 103
+++++++++++++++++++++++++++--
2 files changed, 108 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/wl1271.h
b/drivers/net/wireless/wl12xx/wl1271.h
index faa5925..013eabb 100644
*func)
free_irq(wl->irq, wl);
wl1271_unregister_hw(wl);
+
+ platform_driver_unregister(&wl->pinstance->pdriver);
+ kfree(wl->pinstance);
+
wl1271_free_hw(wl);
}
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
but you shouldn't fiddle with the driver structure here. What are you
actually trying to achieve here ? What's pinstance supposed to do ? are
you trying to handle cases where you might have several wl12xx chips on
the same board ? If that's the case you should have several platform
devices, one for each chip. They'll only have different ids.
--
balbi
DefectiveByDesign.org
From: DebBarma, Tarun Kanti <hidden> Date: 2010-08-11 18:52:54
-----Original Message-----
From: Felipe Balbi [mailto:felipe.balbi at nokia.com]
Sent: Thursday, August 12, 2010 12:18 AM
To: DebBarma, Tarun Kanti
Cc: Ohad Ben-Cohen; linux-wireless at vger.kernel.org; linux-
mmc at vger.kernel.org; linux-omap at vger.kernel.org; Ido Yariv; Mark Brown;
linux-arm-kernel at lists.infradead.org; Chikkature Rajashekar, Madhusudhan;
Coelho Luciano (Nokia-MS/Helsinki); akpm at linux-foundation.org; San Mehat;
Quadros Roger (Nokia-MS/Helsinki); Tony Lindgren; Nicolas Pitre; Pandita,
Vikram; Kalle Valo
Subject: Re: [PATCH v4 3/8] wireless: wl1271: add platform driver to get
board data
hi,
On Wed, Aug 11, 2010 at 08:42:18PM +0200, ext DebBarma, Tarun Kanti wrote:
What about checking for pdev here before extracting pdata?
why ? if probe is called pdev is guaranteed to be valid, no ?
True; however if we go by that argument than we can also assume pdata is valid, so that we would not need the below check.
Still, I would go ahead and find out if there is any scenario where pdev can go wrong during device registration. Thanks.
but you shouldn't fiddle with the driver structure here. What are you
actually trying to achieve here ? What's pinstance supposed to do ? are
you trying to handle cases where you might have several wl12xx chips on
the same board ? If that's the case you should have several platform
devices, one for each chip. They'll only have different ids.
--
balbi
DefectiveByDesign.org
From: Felipe Balbi <hidden> Date: 2010-08-11 18:57:25
Hi,
On Wed, Aug 11, 2010 at 08:52:54PM +0200, ext DebBarma, Tarun Kanti wrote:
True; however if we go by that argument than we can also assume pdata
is valid, so that we would not need the below check.
of course not. You can have devices that just play well with default
values or devices where you don't need the flexibility of platform data.
That's why we check.
platform_device pointers on the other hand, are guaranteed to be always
true, if it isn't then you should oops, you deserve to oops because
something is really really wrong.
Still, I would go ahead and find out if there is any scenario where
pdev can go wrong during device registration. Thanks.
if that scenario ever happens, it's either a bug on your implementation
or driver-core. In both cases you deserve to oops so we catch such
problems at early stages.
--
balbi
DefectiveByDesign.org
From: DebBarma, Tarun Kanti <hidden> Date: 2010-08-11 19:27:18
-----Original Message-----
From: Felipe Balbi [mailto:felipe.balbi at nokia.com]
Sent: Thursday, August 12, 2010 12:27 AM
To: DebBarma, Tarun Kanti
Cc: Balbi Felipe (Nokia-MS/Helsinki); Ohad Ben-Cohen; linux-
wireless at vger.kernel.org; linux-mmc at vger.kernel.org; linux-
omap at vger.kernel.org; Ido Yariv; Mark Brown; linux-arm-
kernel at lists.infradead.org; Chikkature Rajashekar, Madhusudhan; Coelho
Luciano (Nokia-MS/Helsinki); akpm at linux-foundation.org; San Mehat; Quadros
Roger (Nokia-MS/Helsinki); Tony Lindgren; Nicolas Pitre; Pandita, Vikram;
Kalle Valo
Subject: Re: [PATCH v4 3/8] wireless: wl1271: add platform driver to get
board data
Hi,
On Wed, Aug 11, 2010 at 08:52:54PM +0200, ext DebBarma, Tarun Kanti wrote:
quoted
True; however if we go by that argument than we can also assume pdata
is valid, so that we would not need the below check.
of course not. You can have devices that just play well with default
values or devices where you don't need the flexibility of platform data.
That's why we check.
platform_device pointers on the other hand, are guaranteed to be always
true, if it isn't then you should oops, you deserve to oops because
something is really really wrong.
Sounds perfect!
What that means is _probe() function makes sense only for cases where we have valid platform data because we are returning right at the top if pdata is not valid. If this is the case I was curious to know why not framework make another check for valid pdata before calling _probe() instead of coming all the way to _probe() and then returning!
quoted
Still, I would go ahead and find out if there is any scenario where
pdev can go wrong during device registration. Thanks.
if that scenario ever happens, it's either a bug on your implementation
or driver-core. In both cases you deserve to oops so we catch such
problems at early stages.
--
balbi
DefectiveByDesign.org
but you shouldn't fiddle with the driver structure here. What are you
actually trying to achieve here ? What's pinstance supposed to do ? are you
trying to handle cases where you might have several wl12xx chips on the same
board ?
Yes.
Think of several wl1271 devices, each of which is represented by two
devices; an SDIO function, and a platform device. The SDIO function
stands for a specific MMC controller the device is hardwired to, and
the platform device stands for the external irq line that the device
is hardwired to.
We must couple the SDIO function with the correct platform device,
otherwise it will start getting the wrong interrupts. So after the
SDIO function is probed, it registers a platform driver with the
unique name that represents the platform device that it is coupled
with. When the platform device is probed, it needs to deliver its
platform data info to the specific SDIO function that it is bound
with.
To avoid using some global data structure in that driver, we allocate
a unique platform driver per each device, which makes it possible for
the platform driver probe to find the SDIO function context by means
of container_of.
Alternatively, we can also use some global structure in that file,
most probably idr, which would also make it possible to reach the SDIO
function contexts without going through the driver structure. The idr
indexes would then be the MMC controller index, which should match
the platform device id.
I'll try this out, it might actually look nicer.
Thanks,
Ohad.
If that's the case you should have several platform devices, one for
each chip. They'll only have different ids.
--
balbi
DefectiveByDesign.org
From: Russell King - ARM Linux <hidden> Date: 2010-08-11 21:21:56
On Thu, Aug 12, 2010 at 12:22:54AM +0530, DebBarma, Tarun Kanti wrote:
True; however if we go by that argument than we can also assume pdata
is valid, so that we would not need the below check.
If pdev was ever NULL in a probe function, the kernel deserves to OOPS
so that you have a backtrace to fix the bugger.
Basically, a probe function is only called when the driver finds a
matching device to bind to - so the device _must_ already exist and
be valid. It's basically guaranteed. So checking for a NULL pdev is
not only a waste of space, it's a waste of CPU time and developer time
writing the check as well.
On the other hand, platform data passed in via a platform device _is_
liable to be NULL if whatever created the platform device didn't
set the platform data up. So we can't guarantee that the platform
data will exist. So a NULL check is appropriate here.
So, if an API in normal operation requires non-NULL data to be passed,
don't bother checking for a NULL pointer. If you're passed a NULL
pointer in this situation, you deserve to OOPS so you get a backtrace
to fix the problem rather than silently ignoring the problem.
From: Russell King - ARM Linux <hidden> Date: 2010-08-11 21:25:13
On Thu, Aug 12, 2010 at 12:57:18AM +0530, DebBarma, Tarun Kanti wrote:
quoted
-----Original Message-----
From: Felipe Balbi [mailto:felipe.balbi at nokia.com]
Sent: Thursday, August 12, 2010 12:27 AM
To: DebBarma, Tarun Kanti
Cc: Balbi Felipe (Nokia-MS/Helsinki); Ohad Ben-Cohen; linux-
wireless at vger.kernel.org; linux-mmc at vger.kernel.org; linux-
omap at vger.kernel.org; Ido Yariv; Mark Brown; linux-arm-
kernel at lists.infradead.org; Chikkature Rajashekar, Madhusudhan; Coelho
Luciano (Nokia-MS/Helsinki); akpm at linux-foundation.org; San Mehat; Quadros
Roger (Nokia-MS/Helsinki); Tony Lindgren; Nicolas Pitre; Pandita, Vikram;
Kalle Valo
Subject: Re: [PATCH v4 3/8] wireless: wl1271: add platform driver to get
board data
Hi,
On Wed, Aug 11, 2010 at 08:52:54PM +0200, ext DebBarma, Tarun Kanti wrote:
quoted
True; however if we go by that argument than we can also assume pdata
is valid, so that we would not need the below check.
of course not. You can have devices that just play well with default
values or devices where you don't need the flexibility of platform data.
That's why we check.
platform_device pointers on the other hand, are guaranteed to be always
true, if it isn't then you should oops, you deserve to oops because
something is really really wrong.
Sounds perfect!
What that means is _probe() function makes sense only for cases where we
have valid platform data because we are returning right at the top if
pdata is not valid. If this is the case I was curious to know why not
framework make another check for valid pdata before calling _probe()
instead of coming all the way to _probe() and then returning!
Platform devices are not for passing platform data around - they're for
declaring platform hardware devices that we want drivers to handle -
and it depends on the driver whether having platform data is appropriate
or not.
This proposal is, IMHO, abusing the platform device/driver support to
achieve its own goals. I've outlined a far simpler and easiler solution
which avoids this kind of abuse, and given suggestions on how to extend
it to support multiple instances.
Hi Ohad,
On Wed, Aug 11, 2010 at 10:10 PM, Ohad Ben-Cohen [off-list ref] wrote:
Think of several wl1271 devices, each of which is represented by two
devices; an SDIO function, and a platform device. The SDIO function
stands for a specific MMC controller the device is hardwired to, and
the platform device stands for the external irq line that the device
is hardwired to.
my view on this is that you're hunting the false problem here. I
wonder how likely it is that two hardwired WL12xx chips are there on
the same board.
It's theoretically possible that there's one hardwired device and one
hot-pluggable one but the latter one can't have platform_data anyway.
So in my opinion, we either should keep things simple and consider 2
hardwired WL12XX's to be a very very unlikely case, or we'll end up
discussing board_info once again.
Thanks,
Vitaly
On Thu, Aug 12, 2010 at 12:25 AM, Russell King - ARM Linux
[off-list ref] wrote:
On Thu, Aug 12, 2010 at 12:57:18AM +0530, DebBarma, Tarun Kanti wrote:
quoted
quoted
-----Original Message-----
From: Felipe Balbi [mailto:felipe.balbi at nokia.com]
Sent: Thursday, August 12, 2010 12:27 AM
To: DebBarma, Tarun Kanti
Cc: Balbi Felipe (Nokia-MS/Helsinki); Ohad Ben-Cohen; linux-
wireless at vger.kernel.org; linux-mmc at vger.kernel.org; linux-
omap at vger.kernel.org; Ido Yariv; Mark Brown; linux-arm-
kernel at lists.infradead.org; Chikkature Rajashekar, Madhusudhan; Coelho
Luciano (Nokia-MS/Helsinki); akpm at linux-foundation.org; San Mehat; Quadros
Roger (Nokia-MS/Helsinki); Tony Lindgren; Nicolas Pitre; Pandita, Vikram;
Kalle Valo
Subject: Re: [PATCH v4 3/8] wireless: wl1271: add platform driver to get
board data
Hi,
On Wed, Aug 11, 2010 at 08:52:54PM +0200, ext DebBarma, Tarun Kanti wrote:
quoted
True; however if we go by that argument than we can also assume pdata
is valid, so that we would not need the below check.
of course not. You can have devices that just play well with default
values or devices where you don't need the flexibility of platform data.
That's why we check.
platform_device pointers on the other hand, are guaranteed to be always
true, if it isn't then you should oops, you deserve to oops because
something is really really wrong.
Sounds perfect!
What that means is _probe() function makes sense only for cases where we
have valid platform data because we are returning right at the top if
pdata is not valid. If this is the case I was curious to know why not
framework make another check for valid pdata before calling _probe()
instead of coming all the way to _probe() and then returning!
Platform devices are not for passing platform data around - they're for
declaring platform hardware devices that we want drivers to handle -
and it depends on the driver whether having platform data is appropriate
or not.
This proposal is, IMHO, abusing the platform device/driver support to
achieve its own goals. ?I've outlined a far simpler and easiler solution
which avoids this kind of abuse, and given suggestions on how to extend
it to support multiple instances.
Thanks, Russell.
I'll give your suggestion a spin when I get back (I'm going to be away
for some time now with no devices with me).
Hi Vitaly,
On Thu, Aug 12, 2010 at 12:34 AM, Vitaly Wool [off-list ref] wrote:
On Wed, Aug 11, 2010 at 10:10 PM, Ohad Ben-Cohen [off-list ref] wrote:
quoted
Think of several wl1271 devices, each of which is represented by two
devices; an SDIO function, and a platform device. The SDIO function
stands for a specific MMC controller the device is hardwired to, and
the platform device stands for the external irq line that the device
is hardwired to.
my view on this is that you're hunting the false problem here. I
wonder how likely it is that two hardwired WL12xx chips are there on
the same board.
It's theoretically possible that there's one hardwired device and one
hot-pluggable one but the latter one can't have platform_data anyway.
We do have strange-looking setups with two hardwired devices
(especially on new bring-ups), so it would be nice not to limit the
driver to a single device. But I agree, it's far from being important.
Thanks,
Ohad.
So in my opinion, we either should keep things simple and consider 2
hardwired WL12XX's to be a very very unlikely case, or we'll end up
discussing board_info once again.
Thanks,
? Vitaly
? ? ? return IRQ_HANDLED;
?}
-static void wl1271_spi_set_power(struct wl1271 *wl, bool enable)
+static int wl1271_spi_set_power(struct wl1271 *wl, bool enable)
?{
? ? ? if (wl->set_power)
? ? ? ? ? ? ? wl->set_power(enable);
+
+ ? ? return 0;
?}
?static struct wl1271_if_operations spi_ops = {
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html
From: Felipe Balbi <hidden> Date: 2010-08-12 05:21:26
Hi,
On Wed, Aug 11, 2010 at 09:27:18PM +0200, ext DebBarma, Tarun Kanti wrote:
Sounds perfect!
What that means is _probe() function makes sense only for cases where
we have valid platform data because we are returning right at the top
if pdata is not valid. If this is the case I was curious to know why
not framework make another check for valid pdata before calling
_probe() instead of coming all the way to _probe() and then returning!
because pdata is completely optional. pdev OTOH isn't. Without a matched
platform_device, your probe() won't get called.
--
balbi
DefectiveByDesign.org
From: Felipe Balbi <hidden> Date: 2010-08-12 05:27:33
Hi,
On Wed, Aug 11, 2010 at 10:10:38PM +0200, ext Ohad Ben-Cohen wrote:
Think of several wl1271 devices, each of which is represented by two
devices; an SDIO function, and a platform device. The SDIO function
stands for a specific MMC controller the device is hardwired to, and
the platform device stands for the external irq line that the device
is hardwired to.
We must couple the SDIO function with the correct platform device,
otherwise it will start getting the wrong interrupts. So after the
SDIO function is probed, it registers a platform driver with the
unique name that represents the platform device that it is coupled
with. When the platform device is probed, it needs to deliver its
platform data info to the specific SDIO function that it is bound
with.
To avoid using some global data structure in that driver, we allocate
a unique platform driver per each device, which makes it possible for
the platform driver probe to find the SDIO function context by means
of container_of.
Alternatively, we can also use some global structure in that file,
most probably idr, which would also make it possible to reach the SDIO
function contexts without going through the driver structure. The idr
indexes would then be the MMC controller index, which should match
the platform device id.
I'll try this out, it might actually look nicer.
you need no globals and you don't need to fiddle with driver structures
either.
Just make an sdio driver that platform_device_alloc() and pass in a
platform_data, builds struct resources etc for each device found on the
sdio bus. Similarly for the SPI-connected wifi chip.
You should never fiddle with internal structures. If you want to look at
an example implementation, take a look at drivers/mfd/twl-core.c, there
we use an i2c driver to instantiate several platform_devices. Nobody
fiddles with driver structures and all the drivers are logically
separated into their own source files.
--
balbi
DefectiveByDesign.org
On Thu, Aug 12, 2010 at 12:25 AM, Russell King - ARM Linux
[off-list ref] wrote:
I've outlined a far simpler and easiler solution
which avoids this kind of abuse, and given suggestions on how to extend
it to support multiple instances.
From: Russell King - ARM Linux <hidden> Date: 2010-08-12 09:55:46
On Thu, Aug 12, 2010 at 09:40:19AM +0300, Ohad Ben-Cohen wrote:
On Thu, Aug 12, 2010 at 12:25 AM, Russell King - ARM Linux
[off-list ref] wrote:
quoted
I've outlined a far simpler and easiler solution
which avoids this kind of abuse, and given suggestions on how to extend
it to support multiple instances.
On Thursday, August 12, 2010, Russell King - ARM Linux
[off-list ref] wrote:
On Thu, Aug 12, 2010 at 09:40:19AM +0300, Ohad Ben-Cohen wrote:
quoted
On Thu, Aug 12, 2010 at 12:25 AM, Russell King - ARM Linux
[off-list ref] wrote:
quoted
I've outlined a far simpler and easiler solution
which avoids this kind of abuse, and given suggestions on how to extend
it to support multiple instances.
From: DebBarma, Tarun Kanti <hidden> Date: 2010-08-16 04:21:04
-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Thursday, August 12, 2010 2:55 AM
To: DebBarma, Tarun Kanti
Cc: felipe.balbi at nokia.com; Ohad Ben-Cohen; Kalle Valo; Pandita, Vikram;
akpm at linux-foundation.org; Quadros Roger (Nokia-MS/Helsinki); Tony
Lindgren; linux-wireless at vger.kernel.org; Mark Brown; linux-
mmc at vger.kernel.org; Nicolas Pitre; San Mehat; Chikkature Rajashekar,
Madhusudhan; Coelho Luciano (Nokia-MS/Helsinki); linux-
omap at vger.kernel.org; Ido Yariv; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH v4 3/8] wireless: wl1271: add platform driver to get
board data
On Thu, Aug 12, 2010 at 12:57:18AM +0530, DebBarma, Tarun Kanti wrote:
quoted
quoted
-----Original Message-----
From: Felipe Balbi [mailto:felipe.balbi at nokia.com]
Sent: Thursday, August 12, 2010 12:27 AM
To: DebBarma, Tarun Kanti
Cc: Balbi Felipe (Nokia-MS/Helsinki); Ohad Ben-Cohen; linux-
wireless at vger.kernel.org; linux-mmc at vger.kernel.org; linux-
omap at vger.kernel.org; Ido Yariv; Mark Brown; linux-arm-
kernel at lists.infradead.org; Chikkature Rajashekar, Madhusudhan; Coelho
Luciano (Nokia-MS/Helsinki); akpm at linux-foundation.org; San Mehat;
Quadros
quoted
quoted
Roger (Nokia-MS/Helsinki); Tony Lindgren; Nicolas Pitre; Pandita,
Vikram;
quoted
quoted
Kalle Valo
Subject: Re: [PATCH v4 3/8] wireless: wl1271: add platform driver to
get
quoted
quoted
board data
Hi,
On Wed, Aug 11, 2010 at 08:52:54PM +0200, ext DebBarma, Tarun Kanti
wrote:
quoted
quoted
quoted
True; however if we go by that argument than we can also assume pdata
is valid, so that we would not need the below check.
of course not. You can have devices that just play well with default
values or devices where you don't need the flexibility of platform
data.
quoted
quoted
That's why we check.
platform_device pointers on the other hand, are guaranteed to be
always
quoted
quoted
true, if it isn't then you should oops, you deserve to oops because
something is really really wrong.
Sounds perfect!
What that means is _probe() function makes sense only for cases where we
have valid platform data because we are returning right at the top if
pdata is not valid. If this is the case I was curious to know why not
framework make another check for valid pdata before calling _probe()
instead of coming all the way to _probe() and then returning!
Platform devices are not for passing platform data around - they're for
declaring platform hardware devices that we want drivers to handle -
and it depends on the driver whether having platform data is appropriate
or not.
This proposal is, IMHO, abusing the platform device/driver support to
achieve its own goals. I've outlined a far simpler and easiler solution
which avoids this kind of abuse, and given suggestions on how to extend
it to support multiple instances.
I guess I have not put my question in the right perspective. As you have said, platform devices are not meant for passing data around, it appeared to me contradictory when in the probe() we are just checking the pdata and proceeding/returning based upon its value, as if platform device has no other functions. So, the question should have been what else we could do in probe() with platform device other than manipulation of pdata, other than modification of the framework in the real sense.
Anyways, I will go through the suggestions you mentioned. Thanks for the clarifications.