Re: [PATCH 22/51] DMA-API: amba: get rid of separate dma_mask
On Thu, 19 Sep 2013 22:47:01 +0100, Russell King rmk+ker...@arm.linux.org.uk wrote: AMBA Primecell devices always treat streaming and coherent DMA exactly the same, so there's no point in having the masks separated. Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk for the drivers/of/platform.c portion: Acked-by: Grant Likely grant.lik...@linaro.org g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: omap_hsmmc: Enable SDIO IRQ using a GPIO in idle mode.
On Thu, 20 Dec 2012 23:12:12 +0100, Andreas Fenkart
andreas.fenk...@streamunlimited.com wrote:
Without functional clock the omap_hsmmc module can't forward
SDIO IRQs to the system. This patch reconfigures dat1 line
as a gpio while the fclk is off. And uses SDIO IRQ detection of
the module, while fclk is present.
Signed-off-by: Andreas Fenkart andreas.fenk...@streamunlimited.com
---
.../devicetree/bindings/mmc/ti-omap-hsmmc.txt | 42
arch/arm/plat-omap/include/plat/mmc.h |4 +
drivers/mmc/host/omap_hsmmc.c | 219
++--
3 files changed, 247 insertions(+), 18 deletions(-)
diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
index d1b8932..4d57637 100644
--- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
+++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
@@ -24,6 +24,29 @@ One tx and one rx pair is required.
dma-names: DMA request names. These strings correspond 1:1 with
the ordered pairs in dmas. The RX request must be rx and the
TX request must be tx.
+ti,cirq-gpio: When omap_hsmmc module is suspended, its functional
+clock is turned off. Without fclk it can't forward SDIO IRQs to the
+system. For that to happen, it needs to tell the PRCM to restore
+its fclk, which is done through the swakeup line.
+
+ --
+ | PRCM |
+ --
+| ^
+ fclk | | swakeup
+v |
+ --- --
+ -- IRQ -- | hsmmc | -- CIRQ -- | card |
+ --- --
+
+The problem is, that on the AM335x family the swakeup line is
+missing, it has not been routed from the module to the PRCM.
+The way to work around this, is to reconfigure the dat1 line as a
+GPIO upon suspend. Beyond this option you also need to set named
+states default and idle in the .dts file for the pins, using
+pinctrl-single.c. The MMC driver will then then toggle between
+default and idle during the runtime.
+
Examples:
@@ -53,3 +76,22 @@ Examples:
edma 25;
dma-names = tx, rx;
};
+
+[am335x with with gpio for sdio irq]
+
+ mmc1_cirq_pin: pinmux_cirq_pin {
+ pinctrl-single,pins =
+ 0x0f8 0x3f /* MMC0_DAT1 as GPIO2_28 */
+ ;
+ };
+
+ mmc1: mmc@4806 {
+ pinctrl-names = default, idle;
+ pinctrl-0 = mmc1_pins;
+ pinctrl-1 = mmc1_cirq_pin;
+ ti,cirq-gpio = gpio3 28 0;
+ ti,non-removable;
+ bus-width = 4;
+ vmmc-supply = ldo2_reg;
+ vmmc_aux-supply = vmmc;
+ };
Binding looks reasonable.
Reviewed-by: Grant Likely grant.lik...@secretlab.ca
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] mmc: dt: bus-width can be an optional property
On Mon, 21 Jan 2013 19:02:29 +0800, Shawn Guo shawn@linaro.org wrote: None of mmc drivers implements bus-width as a required device tree property. Instead, some drivers like atmel-mci, dw_mmc, sdhci-s3c implement it as an optional one, and will force bus width to be 1 when the property is absent. Let's change the common binding to reflect what the drivers are usually doing. Signed-off-by: Shawn Guo shawn@linaro.org Cc: devicetree-disc...@lists.ozlabs.org Acked-by: Grant Likely grant.lik...@secretlab.ca --- Documentation/devicetree/bindings/mmc/mmc.txt |5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt index a591c67..cef217d 100644 --- a/Documentation/devicetree/bindings/mmc/mmc.txt +++ b/Documentation/devicetree/bindings/mmc/mmc.txt @@ -6,9 +6,6 @@ Interpreted by the OF core: - reg: Registers location and length. - interrupts: Interrupts used by the MMC controller. -Required properties: -- bus-width: Number of data lines, can be 1, 4, or 8 - Card detection: If no property below is supplied, standard SDHCI card detect is used. Only one of the properties in this section should be supplied: @@ -17,6 +14,8 @@ Only one of the properties in this section should be supplied: - non-removable: non-removable slot (like eMMC); assume always present. Optional properties: +- bus-width: Number of data lines, can be 1, 4, or 8. The default + will be 1 if the property is absent. - wp-gpios: Specify GPIOs for write protection, see gpio binding - cd-inverted: when present, polarity on the cd gpio line is inverted - wp-inverted: when present, polarity on the wp gpio line is inverted -- 1.7.9.5 ___ devicetree-discuss mailing list devicetree-disc...@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 3.7.0-rc4 0/4] introduce of_simple_module_id_table macro
On Fri, 16 Nov 2012 13:21:08 +, Srinivas KANDAGATLA
srinivas.kandaga...@st.com wrote:
From: Srinivas Kandagatla srinivas.kandaga...@st.com
This patch series introduces of_simple_module_id_table macro and as an example
uses this macro in 3 files.
Most of the device tree supported drivers have of_device_id table setup with
single compatible entry, this use-case is very simple and common.
#ifdef CONFIG_OF
static struct of_device_id xxx_of_match[] = {
{ .compatible = yyy,zzz },
{ },
};
MODULE_DEVICE_TABLE(of, xxx_of_match);
#endif
This patch adds a macro for this simple type of device table.
Other subsystems like pm, platform, have similar macros in kernel for
simplest cases.
Now the user can just replace the above code with
of_simple_module_id_table(xxx_of_match, yyy,zzz);
There are more than 200+ hits for this type of pattern in the current kernel.
While I like the reduction in lines of source code, I'm not so fond of
the form. There is no easy way to extend the syntax for multiple
entries and it doesn't cover the frequently present .data field. Can you
think of a way to do this that can take a variable number of table
entries?
g.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] dmaengine: add dma_get_channel_caps()
On Fri, Oct 19, 2012 at 3:51 AM, Matt Porter mpor...@ti.com wrote:
Add a dmaengine API to retrieve per channel capabilities.
Currently, only channel ops and SG segment limitations are
implemented caps.
The API is optionally implemented by drivers and when
unimplemented will return a NULL pointer. It is intended
to be executed after a channel has been requested and, if
the channel is intended to be used with slave SG transfers,
then it may only be called after dmaengine_slave_config()
has executed. The slave driver provides parameters such as
burst size and address width which may be necessary for
the dmaengine driver to use in order to properly return SG
segment limit caps.
Suggested-by: Vinod Koul vinod.k...@intel.com
Signed-off-by: Matt Porter mpor...@ti.com
Looks okay to me. Minor comment below...
+/**
+ * dma_get_channel_caps - flush pending transactions to HW
+ * @chan: target DMA channel
+ * @dir: direction of transfer
+ *
+ * Get the channel-specific capabilities. If the dmaengine
+ * driver does not implement per channel capbilities then
+ * NULL is returned.
+ */
+static inline struct dmaengine_chan_caps
+*dma_get_channel_caps(struct dma_chan *chan, enum dma_transfer_direction dir)
Looks to me like the returned pointer be a const. Ditto for the
callback prototype in the dma_device structure.
+{
+ if (chan-device-device_channel_caps)
+ return chan-device-device_channel_caps(chan, dir);
+ return NULL;
+}
+
enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
#ifdef CONFIG_DMA_ENGINE
enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx);
--
1.7.9.5
--
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/
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 3/3] mmc: davinci: get SG segment limits with dma_get_channel_caps()
On Fri, Oct 19, 2012 at 3:51 AM, Matt Porter mpor...@ti.com wrote:
Replace the hardcoded values used to set max_segs/max_seg_size with
a dma_get_channel_caps() query to the dmaengine driver.
Signed-off-by: Matt Porter mpor...@ti.com
Series looks reasonable to me.
Reviewed-by: Grant Likely grant.lik...@secretlab.ca
---
drivers/mmc/host/davinci_mmc.c| 66
+
include/linux/platform_data/mmc-davinci.h |3 --
2 files changed, 21 insertions(+), 48 deletions(-)
diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index f5d46ea..d1efacc 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -145,18 +145,6 @@
/* MMCSD Init clock in Hz in opendrain mode */
#define MMCSD_INIT_CLOCK 20
-/*
- * One scatterlist dma segment is at most MAX_CCNT rw_threshold units,
- * and we handle up to MAX_NR_SG segments. MMC_BLOCK_BOUNCE kicks in only
- * for drivers with max_segs == 1, making the segments bigger (64KB)
- * than the page or two that's otherwise typical. nr_sg (passed from
- * platform data) == 16 gives at least the same throughput boost, using
- * EDMA transfer linkage instead of spending CPU time copying pages.
- */
-#define MAX_CCNT ((1 16) - 1)
-
-#define MAX_NR_SG 16
-
static unsigned rw_threshold = 32;
module_param(rw_threshold, uint, S_IRUGO);
MODULE_PARM_DESC(rw_threshold,
@@ -217,8 +205,6 @@ struct mmc_davinci_host {
u8 version;
/* for ns in one cycle calculation */
unsigned ns_in_one_cycle;
- /* Number of sg segments */
- u8 nr_sg;
#ifdef CONFIG_CPU_FREQ
struct notifier_block freq_transition;
#endif
@@ -422,16 +408,7 @@ static int mmc_davinci_send_dma_request(struct
mmc_davinci_host *host,
int ret = 0;
if (host-data_dir == DAVINCI_MMC_DATADIR_WRITE) {
- struct dma_slave_config dma_tx_conf = {
- .direction = DMA_MEM_TO_DEV,
- .dst_addr = host-mem_res-start + DAVINCI_MMCDXR,
- .dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
- .dst_maxburst =
- rw_threshold / DMA_SLAVE_BUSWIDTH_4_BYTES,
- };
chan = host-dma_tx;
- dmaengine_slave_config(host-dma_tx, dma_tx_conf);
-
desc = dmaengine_prep_slave_sg(host-dma_tx,
data-sg,
host-sg_len,
@@ -444,16 +421,7 @@ static int mmc_davinci_send_dma_request(struct
mmc_davinci_host *host,
goto out;
}
} else {
- struct dma_slave_config dma_rx_conf = {
- .direction = DMA_DEV_TO_MEM,
- .src_addr = host-mem_res-start + DAVINCI_MMCDRR,
- .src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
- .src_maxburst =
- rw_threshold / DMA_SLAVE_BUSWIDTH_4_BYTES,
- };
chan = host-dma_rx;
- dmaengine_slave_config(host-dma_rx, dma_rx_conf);
-
desc = dmaengine_prep_slave_sg(host-dma_rx,
data-sg,
host-sg_len,
@@ -1166,6 +1134,7 @@ static int __init davinci_mmcsd_probe(struct
platform_device *pdev)
struct resource *r, *mem = NULL;
int ret = 0, irq = 0;
size_t mem_size;
+ struct dmaengine_chan_caps *dma_chan_caps;
/* REVISIT: when we're fully converted, fail if pdata is NULL */
@@ -1215,12 +1184,6 @@ static int __init davinci_mmcsd_probe(struct
platform_device *pdev)
init_mmcsd_host(host);
- if (pdata-nr_sg)
- host-nr_sg = pdata-nr_sg - 1;
-
- if (host-nr_sg MAX_NR_SG || !host-nr_sg)
- host-nr_sg = MAX_NR_SG;
-
host-use_dma = use_dma;
host-mmc_irq = irq;
host-sdio_irq = platform_get_irq(pdev, 1);
@@ -1249,14 +1212,27 @@ static int __init davinci_mmcsd_probe(struct
platform_device *pdev)
mmc-caps |= pdata-caps;
mmc-ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
- /* With no iommu coalescing pages, each phys_seg is a hw_seg.
-* Each hw_seg uses one EDMA parameter RAM slot, always one
-* channel and then usually some linked slots.
-*/
- mmc-max_segs = MAX_NR_SG;
+ {
+ struct dma_slave_config dma_txrx_conf = {
+ .src_addr = host-mem_res-start + DAVINCI_MMCDRR,
+ .src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
+ .src_maxburst =
+ rw_threshold / DMA_SLAVE_BUSWIDTH_4_BYTES,
+ .dst_addr = host-mem_res-start + DAVINCI_MMCDXR
Re: [RFC PATCH 1/3] dmaengine: add dma_get_channel_caps()
On Fri, Oct 19, 2012 at 3:51 AM, Matt Porter mpor...@ti.com wrote:
Add a dmaengine API to retrieve per channel capabilities.
Currently, only channel ops and SG segment limitations are
implemented caps.
The API is optionally implemented by drivers and when
unimplemented will return a NULL pointer. It is intended
to be executed after a channel has been requested and, if
the channel is intended to be used with slave SG transfers,
then it may only be called after dmaengine_slave_config()
has executed. The slave driver provides parameters such as
burst size and address width which may be necessary for
the dmaengine driver to use in order to properly return SG
segment limit caps.
Suggested-by: Vinod Koul vinod.k...@intel.com
Signed-off-by: Matt Porter mpor...@ti.com
---
include/linux/dmaengine.h | 52
+
1 file changed, 52 insertions(+)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 11d9e25..0181887 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -371,6 +371,38 @@ struct dma_slave_config {
unsigned int slave_id;
};
+enum dmaengine_apis {
+ DMAENGINE_MEMCPY= 0x0001,
+ DMAENGINE_XOR = 0x0002,
+ DMAENGINE_XOR_VAL = 0x0004,
+ DMAENGINE_PQ= 0x0008,
+ DMAENGINE_PQ_VAL= 0x0010,
+ DMAENGINE_MEMSET= 0x0020,
+ DMAENGINE_SLAVE = 0x0040,
+ DMAENGINE_CYCLIC= 0x0080,
+ DMAENGINE_INTERLEAVED = 0x0100,
+ DMAENGINE_SG= 0x0200,
+};
Actually, one more comment. Why the new enum? Why can't the
dma_transaction_type enum be used directly along with dma_cap_mask_t?
+
+/* struct dmaengine_chan_caps - expose capability of a channel
+ * Note: each channel can have same or different capabilities
+ *
+ * This primarily classifies capabilities into
+ * a) APIs/ops supported
+ * b) channel physical capabilities
+ *
+ * @ops: or'ed api capability
+ * @seg_nr: maximum number of SG segments supported on a SG/SLAVE
+ * channel (0 for no maximum or not a SG/SLAVE channel)
+ * @seg_len: maximum length of SG segments supported on a SG/SLAVE
+ * channel (0 for no maximum or not a SG/SLAVE channel)
+ */
+struct dmaengine_chan_caps {
+ enum dmaengine_apis ops;
+ int seg_nr;
+ int seg_len;
+};
+
static inline const char *dma_chan_name(struct dma_chan *chan)
{
return dev_name(chan-dev-device);
@@ -534,6 +566,7 @@ struct dma_tx_state {
* struct with auxiliary transfer status information, otherwise the call
* will just return a simple status code
* @device_issue_pending: push pending transactions to hardware
+ * @device_channel_caps: return the channel capabilities
*/
struct dma_device {
@@ -602,6 +635,8 @@ struct dma_device {
dma_cookie_t cookie,
struct dma_tx_state *txstate);
void (*device_issue_pending)(struct dma_chan *chan);
+ struct dmaengine_chan_caps *(*device_channel_caps)(
+ struct dma_chan *chan, enum dma_transfer_direction direction);
};
static inline int dmaengine_device_control(struct dma_chan *chan,
@@ -969,6 +1004,23 @@ dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t
last, dma_cookie_t used,
}
}
+/**
+ * dma_get_channel_caps - flush pending transactions to HW
+ * @chan: target DMA channel
+ * @dir: direction of transfer
+ *
+ * Get the channel-specific capabilities. If the dmaengine
+ * driver does not implement per channel capbilities then
+ * NULL is returned.
+ */
+static inline struct dmaengine_chan_caps
+*dma_get_channel_caps(struct dma_chan *chan, enum dma_transfer_direction dir)
+{
+ if (chan-device-device_channel_caps)
+ return chan-device-device_channel_caps(chan, dir);
+ return NULL;
+}
+
enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
#ifdef CONFIG_DMA_ENGINE
enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx);
--
1.7.9.5
--
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/
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc 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/4] MMC: omap_hsmmc: set platform data after probe from DT node
On Sat, Oct 13, 2012 at 9:05 AM, Daniel Mack zon...@gmail.com wrote:
Hi Grant,
On 13.10.2012 02:05, Grant Likely wrote:
Balaji T K balaj...@ti.com wrote:
On Friday 12 October 2012 08:44 PM, Daniel Mack wrote:
On 12.10.2012 16:56, Balaji T K wrote:
On Friday 12 October 2012 07:59 PM, Daniel Mack wrote:
On 12.10.2012 12:58, Daniel Mack wrote:
diff --git a/drivers/mmc/host/omap_hsmmc.c
b/drivers/mmc/host/omap_hsmmc.c
index 19ccb59..4b70823 100644 ---
a/drivers/mmc/host/omap_hsmmc.c +++
b/drivers/mmc/host/omap_hsmmc.c @@ -1728,6 +1728,7 @@
static int __devinit omap_hsmmc_probe(struct
platform_device *pdev)
const u16 *offsetp = match-data; pdata-reg_offset =
*offsetp; } +pdev-dev.platform_data = pdata; }
if (pdata == NULL) {
FWIW, this is the Oops I see without this patch:
Hi, Shouldn't pdev-dev.platform_data be set to NULL on _remove
?
Why?
To make sure on second insmod it is NULL, When built as module, So
that of_get_hsmmc_pdata is called to create pdata.
Actually the driver should *never* modify the value of
dev-platform_data. Ever.
That's interesting, because many drivers do this, especially since they
were converted to DT probing. Mostly because that way, the platform data
logic in callback functions can be reused, and often the platform
specific data is only stored in pdata and taken from there during the
lifetime of a device.
Is there any particular reason why this approach is frowned upon?
Yes. The platform data pointer is owned by the code that registered
the platform device, not by the device driver. Some drivers do it, but
it is definitely illegal. I should add code to the platform bus core
code to throw a warning to any drivers that do that. It is a problem
because it becomes easy to mess up the lifetime model of device data,
particularly when it comes to unbinding/rebinding devices. For
example, if a driver changes the pdata pointer and then gets unbound,
then there will be a stale pdata pointer that may point to either
incorrect data or a data structure that is no longer allocated. You
could argue that it is fine if the driver is smart about how it cleans
up after itself, but in my experience driver authors rarely get it
correct and it results in a lot more code than is necessary. It is far
better for the driver to either grab all the data it needs out of
pdata at .probe() time, or to keep a copy of the 'correct' pdata
(correct depending on where the device retrieved it's data) in it's
private data structure instead of modifying the device.
Make a copy instead.
A copy of what exactly? Of all members of the legacy pdata you mean?
Yes. If the driver directly accesses the pdata structure outside of
the .probe() hook, then it should be modified to either copy the pdata
into the driver's private data structure, or it should copy the
pointer to the pdata so that OF usage can allocate one itself. For
example:
somedriver_probe(struct platform_device *pdev)
{
struct somedriver_private *somedriver;
somedriver = devm_kzalloc(sizeof (*somedriver), GFP_KERNEL);
somedriver-pdata = pdev-platform_data;
if (OF)
somedriver-pdata = devm_kzalloc(sizeof
(*somedriver-pdata), GFP_KERNEL);
}
The bonus with using devm_kzalloc is the driver doesn't even need to
do anything special to undo these allocations on failure or release.
:-)
g.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc 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/4] MMC: omap_hsmmc: set platform data after probe from DT node
Daniel Mack zon...@gmail.com wrote:
On 13.10.2012 10:48, Grant Likely wrote:
somedriver_probe(struct platform_device *pdev)
{
struct somedriver_private *somedriver;
somedriver = devm_kzalloc(sizeof (*somedriver), GFP_KERNEL);
somedriver-pdata = pdev-platform_data;
if (OF)
somedriver-pdata = devm_kzalloc(sizeof
(*somedriver-pdata), GFP_KERNEL);
}
The bonus with using devm_kzalloc is the driver doesn't even need to
do anything special to undo these allocations on failure or release.
:-)
Ok, understood. Will keep an eye on this in the future. Thanks again
for
the explanation.
For this particular driver, this means that both my and Balaji's ways
of
fixing this are wrong?
Balaji's patch looks fine since it uses a local copy of the platform data
structure. It appears that the driver is already creating a local copy and
Balaji is just bug fixing call sites that aren't using it yet.
g.
--
Grant Likely, P.Eng.
Secret Lab Technologies Ltd.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc 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/4] MMC: omap_hsmmc: set platform data after probe from DT node
Balaji T K balaj...@ti.com wrote:
On Friday 12 October 2012 08:44 PM, Daniel Mack wrote:
On 12.10.2012 16:56, Balaji T K wrote:
On Friday 12 October 2012 07:59 PM, Daniel Mack wrote:
On 12.10.2012 12:58, Daniel Mack wrote:
diff --git a/drivers/mmc/host/omap_hsmmc.c
b/drivers/mmc/host/omap_hsmmc.c
index 19ccb59..4b70823 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1728,6 +1728,7 @@ static int __devinit omap_hsmmc_probe(struct
platform_device *pdev)
const u16 *offsetp = match-data;
pdata-reg_offset = *offsetp;
}
+ pdev-dev.platform_data = pdata;
}
if (pdata == NULL) {
FWIW, this is the Oops I see without this patch:
Hi,
Shouldn't pdev-dev.platform_data be set to NULL on _remove ?
Why?
To make sure on second insmod it is NULL, When built as module,
So that of_get_hsmmc_pdata is called to create pdata.
Actually the driver should *never* modify the value of dev-platform_data.
Ever. Make a copy instead.
g.
--
Grant Likely, P.Eng.
Secret Lab Technologies Ltd.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] arm: at91: dt: sam9g25ek add mci support
On Thu, 22 Mar 2012 15:53:01 +0100, Nicolas Ferre nicolas.fe...@atmel.com
wrote:
On 03/21/2012 07:03 PM, ludovic.desroc...@atmel.com :
From: Ludovic Desroches ludovic.desroc...@atmel.com
Signed-off-by: Ludovic Desroches ludovic.desroc...@atmel.com
Signed-off-by: Nicolas Ferre nicolas.fe...@atmel.com
I think you really mean Acked-by. It is only appropriate to add a
Signed-off-by tag when you have actually picked up and either
committed or reposted a patch. When replying to a patch on the
mailing list and giving your okay, the correct tag is either
Acked-by or Reviewed-by (The difference being that Acked-by
infers that you have some authority over the areas touched by the
patch).
See Documentation/SubmittingPatches
g.
---
arch/arm/boot/dts/at91sam9g25ek.dts | 16
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/at91sam9g25ek.dts
b/arch/arm/boot/dts/at91sam9g25ek.dts
index 7a13d09..960de2b 100644
--- a/arch/arm/boot/dts/at91sam9g25ek.dts
+++ b/arch/arm/boot/dts/at91sam9g25ek.dts
@@ -32,6 +32,22 @@
phy-mode = rmii;
status = okay;
};
+
+ mmc0: mmc@f0008000 {
+ status = okay;
+ slot@0 {
+ bus-width = 4;
+ cd-gpios = pioD 15 0;
+ };
+ };
+
+ mmc1: mmc@f000c000 {
+ status = okay;
+ slot@0 {
+ bus-width = 4;
+ cd-gpios = pioD 14 0;
+ };
+ };
};
usb0: ohci@0060 {
--
Nicolas Ferre
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/5] ARM: imx28: add basic dt support
On Tue, 13 Mar 2012 16:47:04 +0800, Dong Aisheng b29...@freescale.com wrote:
From: Dong Aisheng dong.aish...@linaro.org
This patch includes basic dt support which can boot via nfs rootfs.
Signed-off-by: Dong Aisheng dong.aish...@linaro.org
---
Documentation/devicetree/bindings/arm/fsl.txt |4 +
arch/arm/boot/dts/imx28-evk.dts | 31 +
arch/arm/boot/dts/imx28.dtsi | 88
+
arch/arm/mach-mxs/Kconfig |9 +++
arch/arm/mach-mxs/Makefile|1 +
arch/arm/mach-mxs/imx28-dt.c | 67 +++
6 files changed, 200 insertions(+), 0 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/fsl.txt
b/Documentation/devicetree/bindings/arm/fsl.txt
index 54bddda..9f21faf 100644
--- a/Documentation/devicetree/bindings/arm/fsl.txt
+++ b/Documentation/devicetree/bindings/arm/fsl.txt
@@ -1,6 +1,10 @@
Freescale i.MX Platforms Device Tree Bindings
---
+i.MX28 Evaluation Kit
+Required root node properties:
+- compatible = fsl,imx28-evk, fsl,imx28;
+
i.MX51 Babbage Board
Required root node properties:
- compatible = fsl,imx51-babbage, fsl,imx51;
diff --git a/arch/arm/boot/dts/imx28-evk.dts b/arch/arm/boot/dts/imx28-evk.dts
new file mode 100644
index 000..9758dc4
--- /dev/null
+++ b/arch/arm/boot/dts/imx28-evk.dts
@@ -0,0 +1,31 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+/dts-v1/;
+/include/ imx28.dtsi
+
+/ {
+ model = Freescale i.MX28 Evaluation Kit;
+ compatible = fsl,imx28-evk, fsl,imx28;
+
+ memory {
+ device_type = memory;
+ reg = 0x4000 0x0800;
+ };
+
+ ahb@8008 {
+ fec@800f {
+ phy-mode = rmii;
+ local-mac-address = [00 04 9F 01 7D 5B];
Generally a bad idea to put a specific mac address into the device tree.
Better to fill it with zeros. Otherwise all the dev boards will end up using
the same value.
+ status = okay;
+ };
+ };
+};
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
new file mode 100644
index 000..acf0dab
--- /dev/null
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -0,0 +1,88 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+/include/ skeleton.dtsi
+
+/ {
+ #address-cells = 1;
+ #size-cells = 1;
+ interrupt-parent = icoll;
+
+ aliases {
+ serial0 = uart1;
+ };
+
+ cpus {
+ cpu@0 {
+ compatible = arm,arm926ejs;
+ };
+ };
+
+ apb@8000 {
+ compatible = simple-bus;
+ #address-cells = 1;
+ #size-cells = 1;
+ reg = 0x8000 0x8;
+ ranges;
+
+ apbh@8000 {
+ compatible = simple-bus;
+ #address-cells = 1;
+ #size-cells = 1;
+ reg = 0x8000 0x3c900;
+ ranges;
+
+ icoll: interrupt-controller@8000 {
+ compatible = fsl,imx28-icoll;
+ interrupt-controller;
+ #interrupt-cells = 1;
+ reg = 0x8000 0x2000;
+ };
+ };
+
+ apbx@8004 {
+ compatible = simple-bus;
+ #address-cells = 1;
+ #size-cells = 1;
+ reg = 0x8004 0x4;
+ ranges;
+
+ uart1: uart@80074000 {
+ compatible = arm,pl011, arm,primecell;
+ reg = 0x80074000 0x2000;
+ interrupts = 47;
+ };
+ };
What is the purpose of the apbh and apbx busses? Will more device nodes
get added to them later, or does each only contain a single device?
g.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/5] ARM: imx28: add basic dt support
On Tue, 13 Mar 2012 09:59:39 -0500, Zach Sadecki zsade...@itwatchdogs.com
wrote:
On 03/13/2012 09:35 AM, Rob Herring wrote:
+ ahb@8008 {
+ fec@800f {
Use generic names: ethernet@800f
Generic is good, but consistency is better, IMHO. grepping existing dts
files in 3.2.9 finds 6 instances of fec@ and 0 instances of ethernet@
+ uart1: uart@80074000 {
Use generic names: uart1: serial@...
Same comment here, but unfortunately there is already inconsistency in
existing files... 25 instances of serial@ and 35 instances of uart@
No, Rob is correct. The generic names recommended practice is well
established and documented. Expand your grep search to include
arch/powerpc/bot/dts/*.
See section 2.2.2 of ePAPR[1]
[1]https://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.1.pdf
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 2/5] mmc: mxs-mmc: add dt probe support
On Tue, 13 Mar 2012 16:47:05 +0800, Dong Aisheng b29...@freescale.com wrote:
From: Dong Aisheng dong.aish...@linaro.org
Signed-off-by: Dong Aisheng dong.aish...@linaro.org
---
The patch is still using a private way for dma part binding
since the common dma binding is still under discussion.
http://www.spinics.net/lists/linux-omap/msg65528.html
Will update to use common dma binding when it hits mainline.
---
.../devicetree/bindings/mmc/fsl-mxs-mmc.txt| 23 ++
drivers/mmc/host/mxs-mmc.c | 82
+++-
2 files changed, 102 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
b/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
new file mode 100644
index 000..adc1142
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/fsl-mxs-mmc.txt
@@ -0,0 +1,23 @@
+* FREESCALE MXS MMC peripheral
+
+Required properties:
+- compatible : Should be fsl,chip-mmc
+- reg : Should contain registers location and length
+- interrupts : Should contain interrupt.
+ The format is irq_err irq_dma.
+- dma_channel: Should contain the dma channel it uses
Don't use '_' in property names.
The is a generic dma binding being drafted that uses a phandle to the dma
controller and the ability to encode channel numbers. You may want to take
a look at it.
+
+Optional properties:
+- wp-gpios : Specify GPIOs for write protection
+- slot-4bit: Specify 4 bit mode support
+- slot-8bit: Specify 8 bit and 4 bit mode support
+
+Examples:
+mmc1: ssp@8001 {
+ compatible = fsl,imx28-mmc;
+ reg = 0x8001 2000;
+ /* irq_err irq_dma */
+ interrupts = 96 82;
+ dma_channel = 0;
+ slot-8bit;
+};
diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index 382c835..6cf2d17 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -38,6 +38,10 @@
#include linux/gpio.h
#include linux/regulator/consumer.h
#include linux/module.h
+#include linux/of.h
+#include linux/of_device.h
+#include linux/of_gpio.h
+#include linux/slab.h
#include mach/mxs.h
#include mach/common.h
@@ -673,17 +677,79 @@ static bool mxs_mmc_dma_filter(struct dma_chan *chan,
void *param)
return true;
}
+#ifdef CONFIG_OF
+static struct resource * __devinit mxs_mmc_get_of_dmares(
+ struct platform_device *pdev)
+{
+ struct device_node *np = pdev-dev.of_node;
+ struct resource *dmares;
+ int ret;
+
+ if (!np)
+ return NULL;
+
+ dmares = kzalloc(sizeof(*dmares), GFP_KERNEL);
devm_kzalloc()
+ dmares-flags = IORESOURCE_DMA;
+ ret = of_property_read_u32(np, dma_channel, dmares-start);
+ if (ret) {
+ dev_err(pdev-dev, unable to get dmares from dt\n);
+ return NULL;
+ }
+ dmares-end = dmares-start;
+
+ return dmares;
+}
+
+static int __devinit mxs_mmc_get_of_property(struct platform_device *pdev,
+ struct mxs_mmc_platform_data **ppdata)
+{
+ struct device_node *np = pdev-dev.of_node;
+ struct mxs_mmc_platform_data *pdata = *ppdata;
+
+ if (!np)
+ return -ENODEV;
+
+ pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
Ditto
Fix up those comments and you can add my:
Acked-by: Grant Likely grant.lik...@secretlab.ca
+
+ if (of_get_property(np, slot-8bit, NULL))
+ pdata-flags |= SLOTF_8_BIT_CAPABLE;
+
+ if (of_get_property(np, slot-4bit, NULL))
+ pdata-flags |= SLOTF_4_BIT_CAPABLE;
+
+ pdata-wp_gpio = of_get_named_gpio(np, wp-gpios, 0);
+
+ dev_dbg(pdev-dev, wp-gpios %d flags %d\n, pdata-wp_gpio,
+ pdata-flags);
+
+ return 0;
+}
+#else
+static struct resource * __devinit mxs_mmc_get_of_dmares(
+ struct platform_device *pdev)
+{
+ return NULL;
+}
+static inline int mxs_mmc_get_of_property(struct platform_device *pdev,
+ struct mxs_mmc_platform_data *pdata)
+{
+ return -ENODEV;
+}
+#endif
+
static int mxs_mmc_probe(struct platform_device *pdev)
{
struct mxs_mmc_host *host;
struct mmc_host *mmc;
struct resource *iores, *dmares, *r;
- struct mxs_mmc_platform_data *pdata;
+ struct mxs_mmc_platform_data *pdata = NULL;
int ret = 0, irq_err, irq_dma;
dma_cap_mask_t mask;
iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
+ dmares = mxs_mmc_get_of_dmares(pdev);
+ if (dmares == NULL)
+ dmares = platform_get_resource(pdev, IORESOURCE_DMA, 0);
irq_err = platform_get_irq(pdev, 0);
irq_dma = platform_get_irq(pdev, 1);
if (!iores || !dmares || irq_err 0 || irq_dma 0)
@@ -740,7 +806,9 @@ static int mxs_mmc_probe(struct platform_device *pdev)
mmc-caps
Re: [PATCH 1/2] Documentation/gpio.txt: Explain expected pinctrl interaction
On Tue, 21 Feb 2012 11:46:03 +0100, Linus Walleij linus.wall...@linaro.org wrote: On Mon, Feb 20, 2012 at 7:27 AM, Stephen Warren swar...@nvidia.com wrote: Update gpio.txt based on recent discussions regarding interaction with the pinctrl subsystem. Previously, gpio_request() was described as explicitly not performing any required mux setup operations etc. Now, gpio_request() is explicitly as explicitly performing any required mux setup operations where possible. In the case it isn't, platform code is required to have set up any required muxing or other configuration prior to gpio_request() being called, in order to maintain the same semantics. This is achieved by gpiolib drivers calling e.g. pinctrl_request_gpio() in their .request() operation. Signed-off-by: Stephen Warren swar...@nvidia.com Acked-by: Linus Walleij linus.wall...@linaro.org Grant can you take this one? I'd prefer for you to have a look at it as well. I've taken this one, but left the 2nd for Olof. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc 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 3/4] arm/dts: OMAP4: Add mmc controller nodes and board data
On Fri, 24 Feb 2012 15:56:52 +0530, Rajendra Nayak rna...@ti.com wrote:
On Friday 24 February 2012 03:46 PM, T Krishnamoorthy, Balaji wrote:
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 29f4589..9204f60 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -25,6 +25,11 @@
serial1 =uart2;
serial2 =uart3;
serial3 =uart4;
+ mmc1 =mmc1;
+ mmc2 =mmc2;
+ mmc3 =mmc3;
+ mmc4 =mmc4;
+ mmc5 =mmc5;
};
cpus {
@@ -155,5 +160,31 @@
#size-cells =0;
ti,hwmods = i2c4;
};
+
+ mmc1: mmc@1 {
+ compatible = ti,omap4-hsmmc;
+ ti,hwmods = mmc1;
+ ti,dual-volt;
+ };
+
+ mmc2: mmc@2 {
+ compatible = ti,omap4-hsmmc;
+ ti,hwmods = mmc2;
+ };
Hi Rajendra,
Is there a way to control the device registration order,
eMMC connected to mmc2 needs to be registered as /dev/mmcblk0p ...
irrespective of whether SD card is mount or not on mmc1 card slot.
So that bootargs would not have to be modified when filesystem is on eMMC.
I don't know if we can, but even if we could, we take care of the same
bootargs working on two boards (say sdp and panda) *if* on sdp I have my
filesystem on eMMC and on panda I have it on external card.
What happens if I want to use my filesystem on both boards on external
card?
of_alias_get_id() may be able to help you here. It will extract the id
numbering from the /aliases node. That is the safe way to do global
enumeration of devices in the device tree (instead of 'cell-index' which
is strongly discouraged)
g.
+
+ mmc3: mmc@3 {
+ compatible = ti,omap4-hsmmc;
+ ti,hwmods = mmc3;
+ };
+
+ mmc4: mmc@4 {
+ compatible = ti,omap4-hsmmc;
+ ti,hwmods = mmc4;
+ };
+
+ mmc5: mmc@5 {
+ compatible = ti,omap4-hsmmc;
+ ti,hwmods = mmc5;
+ };
};
};
--
1.7.1
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc 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/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()
On Fri, 2 Mar 2012 11:06:15 -0800, Tony Lindgren t...@atomide.com wrote:
* Grant Likely grant.lik...@secretlab.ca [120302 10:16]:
On Fri, 2 Mar 2012 10:08:48 -0800, Tony Lindgren t...@atomide.com wrote:
* Tony Lindgren t...@atomide.com [120302 08:31]:
* Grant Likely grant.lik...@secretlab.ca [120302 01:00]:
On Thu, 01 Mar 2012 10:55:24 -0800, Tony Lindgren t...@atomide.com
wrote:
+
+/**
+ * gpiochip_find_by_name() - iterator for locating a gpio_chip by
name
+ * @name: name of the gpio_chip
+ *
+ * Similar to bus_find_device_by_name. It returns a reference to
the
+ * first gpio_chip with matching name. It ignores NULL and empty
names.
+ * If you need to do something more complex, then use
gpiochip_find.
+ */
+struct gpio_chip *gpiochip_find_by_name(const char *name)
+{
+ if (!name || !strcmp(name, ))
+ return NULL;
+
+ return gpiochip_find((void *)name, match_name);
Nasty cast. Can the signature for gpiochip_find be changed to accept
a (const void *)?
I think so, this too comes from the bus code.
Looks like it can't be const as of_node_to_gpiochip uses it with
a struct device_node * that for of_get_named_gpio_flags comes from
of_parse_phandle_with_args.
Really? It sees to work fine here:
Hmm right you are. I must have missed adding the const to
of_gpiochip_is_match, that's good news :)
Want to do that as a separate patch or want me to fold it in?
I've got it as a separate commit in my gpio/next branch.
g.
---
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d773540..e633a2a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1152,8 +1152,9 @@ EXPORT_SYMBOL_GPL(gpiochip_remove);
* non-zero, this function will return to the caller and not iterate over
any
* more gpio_chips.
*/
-struct gpio_chip *gpiochip_find(void *data,
- int (*match)(struct gpio_chip *chip, void
*data))
+struct gpio_chip *gpiochip_find(const void *data,
+ int (*match)(struct gpio_chip *chip,
+const void *data))
{
struct gpio_chip *chip = NULL;
unsigned long flags;
diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c
index e034b38..bba8121 100644
--- a/drivers/of/gpio.c
+++ b/drivers/of/gpio.c
@@ -229,7 +229,7 @@ void of_gpiochip_remove(struct gpio_chip *chip)
}
/* Private function for resolving node pointer to gpio_chip */
-static int of_gpiochip_is_match(struct gpio_chip *chip, void *data)
+static int of_gpiochip_is_match(struct gpio_chip *chip, const void *data)
{
return chip-of_node == data;
}
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 1ff4e22..5f52690 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -142,9 +142,9 @@ extern int __must_check gpiochip_reserve(int start, int
ngpio);
/* add/remove chips */
extern int gpiochip_add(struct gpio_chip *chip);
extern int __must_check gpiochip_remove(struct gpio_chip *chip);
-extern struct gpio_chip *gpiochip_find(void *data,
+extern struct gpio_chip *gpiochip_find(const void *data,
int (*match)(struct gpio_chip *chip,
-void *data));
+const void *data));
/* Always use the library code for GPIO management calls,
--
email sent from notmuch.vim plugin
--
To unsubscribe from this list: send the line unsubscribe linux-mmc 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] mmc: omap_hsmmc: Convert hsmmc driver to use device tree
On Thu, 23 Feb 2012 17:31:27 +0530, Rajendra Nayak rna...@ti.com wrote:
Define dt bindings for the ti-omap-hsmmc, and adapt
the driver to extract data (which was earlier passed as
platform_data) from device tree.
Signed-off-by: Rajendra Nayak rna...@ti.com
---
.../devicetree/bindings/mmc/ti-omap-hsmmc.txt | 31 +
drivers/mmc/host/omap_hsmmc.c | 68
2 files changed, 99 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
new file mode 100644
index 000..e4fa8f0
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
@@ -0,0 +1,31 @@
+* TI Highspeed MMC host controller for OMAP
+
+The Highspeed MMC Host Controller on TI OMAP family
+provides an interface for MMC, SD, and SDIO types of memory cards.
+
+Required properties:
+- compatible:
+ Should be ti,omap2-hsmmc, for OMAP2/3 controllers
+ Should be ti,omap4-hsmmc, for OMAP4 controllers
+- ti,hwmods: Must be mmcn, n is controller instance starting 1
+- reg : should contain hsmmc registers location and length
+
+Optional properties:
+ti,dual-volt: boolean, supports dual voltage cards
+supply-name-supply: phandle to the regulator device tree node
+supply-name examples are vmmc, vmmc_aux etc
+ti,bus-width: Number of data lines, default assumed is 1 if the property is
missing.
+cd-gpios: GPIOs for card detection
+wp-gpios: GPIOs for write protection
+ti,non-removable: non-removable slot (like eMMC)
Binding looks okay. A couple comments below...
[...]
+static const struct of_device_id omap_mmc_of_match[];
If you move the omap_mmc_of_match[] table up to here then there is no
need for the forward declaration.
+
static int __init omap_hsmmc_probe(struct platform_device *pdev)
{
struct omap_mmc_platform_data *pdata = pdev-dev.platform_data;
@@ -1725,6 +1768,14 @@ static int __init omap_hsmmc_probe(struct
platform_device *pdev)
struct omap_hsmmc_host *host = NULL;
struct resource *res;
int ret, irq;
+ const struct of_device_id *match;
+
+ match = of_match_device(omap_mmc_of_match, pdev-dev);
+ if (match) {
+ pdata = of_get_hsmmc_pdata(pdev-dev);
+ if (match-data)
+ pdata-reg_offset = *(u16 *)match-data;
Blech on the ugly cast. It is slightly safer to do thusly:
u16 *offsetp = match-data;
pdata-reg_offset = *offsetp
+ }
if (pdata == NULL) {
dev_err(pdev-dev, Platform Data is missing\n);
@@ -2120,12 +2171,29 @@ static struct dev_pm_ops omap_hsmmc_dev_pm_ops = {
.runtime_resume = omap_hsmmc_runtime_resume,
};
+#if defined(CONFIG_OF)
+static u16 omap4_reg_offset = 0x100;
+
+static const struct of_device_id omap_mmc_of_match[] = {
+ {
+ .compatible = ti,omap2-hsmmc,
+ },
+ {
+ .compatible = ti,omap4-hsmmc,
+ .data = omap4_reg_offset,
+ },
+ {},
+}
+MODULE_DEVICE_TABLE(of, omap_mmc_of_match);
+#endif
+
static struct platform_driver omap_hsmmc_driver = {
.remove = omap_hsmmc_remove,
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
.pm = omap_hsmmc_dev_pm_ops,
+ .of_match_table = of_match_ptr(omap_mmc_of_match),
},
};
--
1.7.1
___
linaro-dev mailing list
linaro-...@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc 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] arm/dts: OMAP3: Add mmc controller nodes and board data
On Fri, 24 Feb 2012 10:49:00 -0800, Tony Lindgren t...@atomide.com wrote:
* Rajendra Nayak rna...@ti.com [120223 19:29]:
On Friday 24 February 2012 12:27 AM, Tony Lindgren wrote:
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -113,5 +113,31 @@
#size-cells =0;
ti,hwmods = i2c3;
};
+
+ mmc1: mmc@1 {
+ compatible = ti,omap2-hsmmc;
+ ti,hwmods = mmc1;
+ ti,dual-volt;
+ };
+
+ mmc2: mmc@2 {
+ compatible = ti,omap2-hsmmc;
+ ti,hwmods = mmc2;
+ };
+
+ mmc3: mmc@3 {
+ compatible = ti,omap2-hsmmc;
+ ti,hwmods = mmc3;
+ };
+
+ mmc4: mmc@4 {
+ compatible = ti,omap2-hsmmc;
+ ti,hwmods = mmc4;
+ };
+
+ mmc5: mmc@5 {
+ compatible = ti,omap2-hsmmc;
+ ti,hwmods = mmc5;
+ };
};
};
These all should all be ti,omap3-hsmmc I guess?
Well, I defined the binding such that both omap2 and omap3
can use the same compatible ti,omap2-hsmmc since there is
no difference in the way they are defined or handled. If thats
confusing, I can have separate compatibles.
Btw, I guess we do the same with a few other re-used IPs as well,
I just checked and mcpsi does the same.
Yeah let's use separate compatibles to avoid confusion.
For omap2 we also have the ti,omap2-mmc in addition to
ti,omap2-hsmmc..
Yes, absolutely use separate compatible values. It is always important
to be specific as to the silicon implementing the IP. The omap3 instance
can also carry the omap2 string in its compatible list:
compatible = ti,omap3-hsmmc, ti,omap2-hsmmc;
g.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc 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/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()
On Thu, 01 Mar 2012 10:55:24 -0800, Tony Lindgren t...@atomide.com wrote:
Currently there is no way for drivers to request a GPIO on a particular
gpio chip. This makes it hard to support multiple GPIO controllers
with dynamic GPIO and interrupt numbering, such as with CONFIG_SPARSE_IRQ.
Make it easier for device drivers to find GPIOs on a specific gpio_chip
by adding two functions: gpiochip_find_by_name() and gpio_find_by_chip_name().
Note that as gpiochip_find() is already exported, we may as well
export gpiochip_find_by_name() too.
How is the device going to know the name of the gpio controller? I
don't particularly like interfaces that find devices by-names since
I don't think device names can be relied to be stable when devices
are instantiated from device tree data.
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Linus Walleij linus.wall...@stericsson.com
Cc: Arnd Bergmann a...@arndb.de
Cc: Rajendra Nayak rna...@ti.com
Signed-off-by: Tony Lindgren t...@atomide.com
---
drivers/gpio/gpiolib.c | 47
include/asm-generic/gpio.h |3 ++-
2 files changed, 49 insertions(+), 1 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 17fdf4b..0e5bf55 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1173,6 +1173,53 @@ struct gpio_chip *gpiochip_find(void *data,
}
EXPORT_SYMBOL_GPL(gpiochip_find);
+static int match_name(struct gpio_chip *chip, void *data)
Even though this is a static, please keep the prefix to avoid
namespace conflicts. gpiochip_match_name()
+{
+ const char *name = data;
This is unnecessary; the void* can be passed directly to sysfs_streq...
but why is sysfs_streq being used here instead of strcmp? This is
not sysfs related code.
+
+ return sysfs_streq(name, chip-label);
+}
+
+/**
+ * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
+ * @name: name of the gpio_chip
+ *
+ * Similar to bus_find_device_by_name. It returns a reference to the
+ * first gpio_chip with matching name. It ignores NULL and empty names.
+ * If you need to do something more complex, then use gpiochip_find.
+ */
+struct gpio_chip *gpiochip_find_by_name(const char *name)
+{
+ if (!name || !strcmp(name, ))
+ return NULL;
+
+ return gpiochip_find((void *)name, match_name);
Nasty cast. Can the signature for gpiochip_find be changed to accept
a (const void *)?
+}
+EXPORT_SYMBOL_GPL(gpiochip_find_by_name);
+
+/**
+ * gpio_find_by_chip_name() - find a gpio on a specific gpio_chip
+ * @chip_name: name of the gpio_chip
+ * @gpio_offset: offset of the gpio on the gpio_chip
+ *
+ * Note that gpiochip_find_by_name returns the first matching
+ * gpio_chip name. For more complex matching, see gpio_find.
+ */
+int gpio_find_by_chip_name(const char *chip_name, unsigned gpio_offset)
+{
+ struct gpio_chip *chip;
+ int gpio, res;
+
+ chip = gpiochip_find_by_name(chip_name);
+ if (!chip)
+ return -ENODEV;
+
+ gpio = chip-base + gpio_offset;
+
+ return gpio;
+}
+EXPORT_SYMBOL_GPL(gpio_find_by_chip_name);
+
/* These optional allocation calls help prevent drivers from stomping
* on each other, and help provide better diagnostics in debugfs.
* They're called even less than the set direction calls.
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 1ff4e22..d7a2100 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -145,7 +145,8 @@ extern int __must_check gpiochip_remove(struct gpio_chip
*chip);
extern struct gpio_chip *gpiochip_find(void *data,
int (*match)(struct gpio_chip *chip,
void *data));
-
+extern struct gpio_chip *gpiochip_find_by_name(const char *name);
+extern int gpio_find_by_chip_name(const char *chip_name, unsigned
gpio_offset);
/* Always use the library code for GPIO management calls,
* or when sleeping may be involved.
--
email sent from notmuch.vim plugin
--
To unsubscribe from this list: send the line unsubscribe linux-mmc 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/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()
On Fri, 2 Mar 2012 10:08:48 -0800, Tony Lindgren t...@atomide.com wrote:
* Tony Lindgren t...@atomide.com [120302 08:31]:
* Grant Likely grant.lik...@secretlab.ca [120302 01:00]:
On Thu, 01 Mar 2012 10:55:24 -0800, Tony Lindgren t...@atomide.com
wrote:
+
+/**
+ * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
+ * @name: name of the gpio_chip
+ *
+ * Similar to bus_find_device_by_name. It returns a reference to the
+ * first gpio_chip with matching name. It ignores NULL and empty names.
+ * If you need to do something more complex, then use gpiochip_find.
+ */
+struct gpio_chip *gpiochip_find_by_name(const char *name)
+{
+ if (!name || !strcmp(name, ))
+ return NULL;
+
+ return gpiochip_find((void *)name, match_name);
Nasty cast. Can the signature for gpiochip_find be changed to accept
a (const void *)?
I think so, this too comes from the bus code.
Looks like it can't be const as of_node_to_gpiochip uses it with
a struct device_node * that for of_get_named_gpio_flags comes from
of_parse_phandle_with_args.
Really? It sees to work fine here:
---
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d773540..e633a2a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1152,8 +1152,9 @@ EXPORT_SYMBOL_GPL(gpiochip_remove);
* non-zero, this function will return to the caller and not iterate over any
* more gpio_chips.
*/
-struct gpio_chip *gpiochip_find(void *data,
- int (*match)(struct gpio_chip *chip, void
*data))
+struct gpio_chip *gpiochip_find(const void *data,
+ int (*match)(struct gpio_chip *chip,
+const void *data))
{
struct gpio_chip *chip = NULL;
unsigned long flags;
diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c
index e034b38..bba8121 100644
--- a/drivers/of/gpio.c
+++ b/drivers/of/gpio.c
@@ -229,7 +229,7 @@ void of_gpiochip_remove(struct gpio_chip *chip)
}
/* Private function for resolving node pointer to gpio_chip */
-static int of_gpiochip_is_match(struct gpio_chip *chip, void *data)
+static int of_gpiochip_is_match(struct gpio_chip *chip, const void *data)
{
return chip-of_node == data;
}
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 1ff4e22..5f52690 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -142,9 +142,9 @@ extern int __must_check gpiochip_reserve(int start, int
ngpio);
/* add/remove chips */
extern int gpiochip_add(struct gpio_chip *chip);
extern int __must_check gpiochip_remove(struct gpio_chip *chip);
-extern struct gpio_chip *gpiochip_find(void *data,
+extern struct gpio_chip *gpiochip_find(const void *data,
int (*match)(struct gpio_chip *chip,
-void *data));
+const void *data));
/* Always use the library code for GPIO management calls,
--
To unsubscribe from this list: send the line unsubscribe linux-mmc 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/1] of_mmc_spi: fix little endian support
On Mon, Jan 30, 2012 at 05:15:29AM +0100, Jean-Christophe PLAGNIOL-VILLARD
wrote:
the voltage_ranges is supposed to switch from big endian to little endian
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: linux-mmc@vger.kernel.org
Applied, thanks.
g.
---
drivers/mmc/host/of_mmc_spi.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/of_mmc_spi.c b/drivers/mmc/host/of_mmc_spi.c
index ab66f24..ede2c64 100644
--- a/drivers/mmc/host/of_mmc_spi.c
+++ b/drivers/mmc/host/of_mmc_spi.c
@@ -109,12 +109,13 @@ struct mmc_spi_platform_data *mmc_spi_get_pdata(struct
spi_device *spi)
goto err_ocr;
}
+
for (i = 0; i num_ranges; i++) {
const int j = i * 2;
u32 mask;
- mask = mmc_vddrange_to_ocrmask(voltage_ranges[j],
-voltage_ranges[j + 1]);
+ mask = mmc_vddrange_to_ocrmask(be32_to_cpu(voltage_ranges[j]),
+be32_to_cpu(voltage_ranges[j +
1]));
if (!mask) {
ret = -EINVAL;
dev_err(dev, OF: voltage-range #%d is invalid\n, i);
--
1.7.7
--
To unsubscribe from this list: send the line unsubscribe linux-mmc 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/1] of_mmc_spi: fix little endian support
On Mon, Jan 30, 2012 at 6:14 AM, Grant Likely grant.lik...@secretlab.ca wrote: On Mon, Jan 30, 2012 at 05:15:29AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: the voltage_ranges is supposed to switch from big endian to little endian Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: linux-mmc@vger.kernel.org Applied, thanks. Actually, this one should go via Chris' tree since it is part of drivers/mmc. So I've dropped it from my tree and change that s-o-b of mine to an a-b. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] mmc: sdhci-s3c: Add device tree support
On Mon, Jan 30, 2012 at 10:51:11AM +0100, Heiko Stübner wrote:
Am Mittwoch, 2. November 2011, 21:36:03 schrieb Thomas Abraham:
Hi Thomas,
in patch 1/6:
+static struct platform_device_id sdhci_s3c_driver_ids[] = {
+ {
+ .name = s3c-sdhci,
+ .driver_data= (kernel_ulong_t)NULL,
+ },
+ {
+ .name = exynos4-sdhci,
+ .driver_data= EXYNOS4_SDHCI_DRV_DATA,
+ },
+};
+MODULE_DEVICE_TABLE(platform, sdhci_s3c_driver_ids);
and in patch 6/6:
+#ifdef CONFIG_OF
+static const struct of_device_id sdhci_s3c_dt_match[] = {
+ { .compatible = samsung,s3c6410-sdhci, },
+ { .compatible = samsung,exynos4210-sdhci,
+ .data = exynos4_sdhci_drv_data },
+ {},
+};
+MODULE_DEVICE_TABLE(of, sdhci_s3c_dt_match);
wouldn't it be better to keep the naming consistent between of and non-of?
I.e. s3c-sdhci vs. s3c6410-sdhci. Since the driver is used for all S3C SoCs
containing hsmmc controllers I think s3c-sdhci would be preferable here.
History has shown that future devices aren't always compatible with earlier
ones. Compatible strings are expected to be specific to an exact device to
reduce the possibility of new hardware breaking assumptions.
Instead, new hardware can either claim compatibility with older
compatible strings (the compatible property in the DT is a list), or
can have the new string added to the match table in the driver;
whichever option makes the most sense.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] mmc: Add OF bindings support for mmc host controller capabilities
On Mon, Nov 07, 2011 at 07:51:26PM +0530, Thomas Abraham wrote: On 5 November 2011 01:27, Olof Johansson o...@lixom.net wrote: On Thu, Nov 03, 2011 at 02:06:02AM +0530, Thomas Abraham wrote: Device nodes representing sd/mmc controllers in a device tree would include mmc host controller capabilities. Add support for parsing of mmc host controller capabilities included in device nodes. Signed-off-by: Thomas Abraham thomas.abra...@linaro.org --- .../devicetree/bindings/mmc/linux-mmc-host.txt | 13 drivers/mmc/core/host.c | 31 include/linux/mmc/host.h | 1 + 3 files changed, 45 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/mmc/linux-mmc-host.txt diff --git a/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt new file mode 100644 index 000..714b2b1 --- /dev/null +++ b/Documentation/devicetree/bindings/mmc/linux-mmc-host.txt @@ -0,0 +1,13 @@ +* Linux MMC Host Controller Capabilities + +The following bindings can be used in a device node to specify any board +specific mmc host controller capabilities. + +- linux,mmc_cap_4_bit_data - Host can do 4-bit transfers +- linux,mmc_cap_mmc_highspeed - Host can do MMC high-speed timing +- linux,mmc_cap_sd_highspeed - Host can do SD high-speed timing +- linux,mmc_cap_needs_poll - Host needs polling for card detection +- linux,mmc_cap_8_bit_data - Host can do 8-bit transfer +- linux,mmc_cap_disable - Host can be disabled and re-enabled to save power +- linux,mmc_cap_nonremovable - Host is connected to nonremovable media +- linux,mmc_cap_erase - Host allows erase/trim commands linux-prefixed properties are a big red flag. The device tree should describe the hardware, not what linux does with the hardware. The vendor-prefixes documentation for device tree bindings includes 'linux' as an option. And I was trying to encode the linux specific host controller capabilities using the above bindings. See previous comments about support-8bit for encoding exactly the same hardware capability in a linux-agnostic way. What the sdhci driver chooses to do with it is up to the driver, and in some cases it means it will set the capabilities flag. Same goes for the other properties. It should not go in as it is implemented in this patch, it needs to be fixed up. Ok. I will remove all these linux specific bindings and replace with a more generic ones. Bindings will be defined for all the linux defined host capabilities. Non-linux platforms can add additional bindings as required. A function to parse such properties from a controller device node could actually be shared among all the mmc/sd host controller drivers in linux. I will redo this patch and submit again. Thanks Olof for your review and comments. This patch appears to be conceptually wrong. Many of these properties are merely static capabilities of each individual device which the driver should already have knowledge of, and be setting the capability bits appropriately on its own. So, you /don't/ want to simply create a 1:1 list between the current linux capability bits (which are subject to change) and the device tree properties (which ideally must not be changed after they are defined). What you need to do is figure out which properties are required to describe the hardware about things that the driver wouldn't already know. For example, 'polling' is something the driver would already know because it is an aspect of the specific device, but 'nonremovable' is a physical characteristic of some boards. 'disable' and the speed timings are also something I would expect the driver to know about and what to set itself. So, only define properties for things that the device-specific driver cannot know for itself; or are the sort of parameters that a multi-device driver is already using for differentiation (ie. appears in pdata instead of directly set by the driver). You should be able to justify the need for each property that gets defined. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] mmc: sdhci-s3c: Add device tree support
On Thu, Nov 03, 2011 at 02:06:03AM +0530, Thomas Abraham wrote:
Add device tree based discovery support for Samsung's sdhci controller
Cc: Ben Dooks ben-li...@fluff.org
Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
---
+Example:
+ sdhci@1253 {
+ compatible = samsung,exynos4210-sdhci;
+ reg = 0x1253 0x100;
+ interrupts = 139;
+ samsung,sdhci-bus-width = 4;
+ linux,mmc_cap_4_bit_data;
Following on from my reply on patch 5, this is an example of exactly
what I'm talking about. This node both sets bus-width to '4', and
sets the 4_bit_data flag. Don't you think that the driver would be
smart enough to set the 4_bit_data flag when the bus width was set to
4?
g.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] drivercore: Add driver probe deferral mechanism
On Fri, Oct 14, 2011 at 9:37 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 13 Oct 2011, Grant Likely wrote: For the deferred case; here is an example of the additional constraint. Consider the following hierarchy: -A +-B | +-C | +-D | +-E +-F +-G dpm_list could be ordered in at least the following ways (depending on exactly when devices get registered). There are many permutation, but children are always be listed after its direct parent. 1) ABECDFG (breadth first) 2) AEBFGCD (breadth first) 3) ABCDEFG (depth first) 4) AEFGBCD (depth first) Now, assume that device B depends on device F, and also assume that there is no way either to express that in the hierarchy or even for the constraint to be known at device registration time (the is exactly the situation we're dealing with on embedded platforms). Only the driver for B knows that it needs a resource provided by F's driver. So, the situation becomes that the ordering of dpm_list must now also be sorted so that non-tree dependencies are also accounted for. Of the list above, only sort order 4 meets the new constraint. The question then becomes, how can the dpm_list get resorted dynamically at runtime to ensure that the new constraints are always met without breaking old ones. Here are some options I can think of: 1) When a deferred probe succeeds, move the deferred device to the end of the dpm_list. Then the new sort order might be one of: 1) AECDFGB 2) AEFGCDB 3) ACDEFGB 4) AEFGCDB However, that approach breaks the guarantee that children appear after their parents (C D appear before B in all cases above) How can a device acquire children before it has a driver? There are a few potential situations in embedded systems (or at least nothing currently prevents it) where platform setup code constructs a device hierarchy without the aid of device drivers, and it is still possible for a parent device to be attached to a driver. IIUC, SPARC creates an entire hierarchy of platform_devices from all the nodes in the OpenFirmware device tree, and any of those devices can be bound to a driver. I don't like that approach, but at the very least it needs to be guarded against. On Fri, Oct 14, 2011 at 9:39 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 13 Oct 2011, Grant Likely wrote: I saw those. I also notice that they are only used by device_move() when reparenting a device (which is another wrinkle I hadn't though about). Reparenting a device becomes problematic if the probe order is also represented in the list. If device_move() gets called, then any implicit probe-order sorting for that device would be lost. I also notice that device_move disregards any children when moving a device, which could also be a problem. Although it looks like the only users of device_move are: drivers/media/video/pvrusb2/pvrusb2-v4l2.c drivers/s390/cio/device.c net/bluetooth/hci_sysfs.c net/bluetooth/rfcomm/tty.c So it may not be significant to adapt. I trust that very little will be needed. I haven't checked the existing callers, but it's reasonable to require that a device being moved not have any children. Yes, that does indeed simplify the issue considerably. Let's do that. So, for this patch, there will be two bits added. First, don't allow deferral on devices with children, and second a successful probe (deferred or otherwise) should always move a device to the end of the dap_list if it doesn't have children to guarantee that the list order satisfies both the hierarchical and probe order. Manjunath, do you think you can prototype this? g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] drivercore: Add driver probe deferral mechanism
On Fri, Oct 14, 2011 at 10:33 AM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 14 Oct 2011, Grant Likely wrote: How can a device acquire children before it has a driver? There are a few potential situations in embedded systems (or at least nothing currently prevents it) where platform setup code constructs a device hierarchy without the aid of device drivers, and it is still possible for a parent device to be attached to a driver. IIUC, SPARC creates an entire hierarchy of platform_devices from all the nodes in the OpenFirmware device tree, and any of those devices can be bound to a driver. I don't like that approach, but at the very least it needs to be guarded against. Do these devices ever require deferred probes? Yes, they very well might. However, I'm happy with the limitation that only leaf devices can take advantage of probe deferral. On Fri, Oct 14, 2011 at 9:39 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 13 Oct 2011, Grant Likely wrote: Yes, that does indeed simplify the issue considerably. Let's do that. So, for this patch, there will be two bits added. First, don't allow deferral on devices with children, and second a successful probe (deferred or otherwise) should always move a device to the end of the dap_list if it doesn't have children to guarantee that the list order satisfies both the hierarchical and probe order. Manjunath, do you think you can prototype this? I don't think the second part needs to be quite so invasive. Certainly drivers that never defer probes shouldn't require anything to be moved. Think about that a minute. Consider a dpm_list of devices: abcDefGh Now assume that D has an implicit dependency on G. If D gets probed first, then it will be deferred until G gets probed and then D will get retried and moved to the end of the list resulting in: abcefGhD Everything is good now for the order that things need to be suspended in. Now lets assume that the drivers get linked into the kernel in a different order (or the modules get loaded in a different order) and G gets probed first, followed by D. No deferral occurred and so no reordering occurs, resulting in: abcDefGh (unchanged) But now suspend is broken because D depends on G, but G will be suspended before D. This looks and smells like a bug to me. In fact, even without using probe deferral it looks like a bug because the dap_list isn't taking into account the fact that there are very likely to be implicit dependencies between devices that are not represented in the device hierarchy (maybe not common in PCs, but certainly is the case for embedded). But, it is also easy to solve by ensuring the dap_list is also probe-order sorted. A deferred probe _should_ move the device to the end of the list. But this needs to happen in the probe routine itself (after it has verified that all the other required devices are present and before it has registered any children) to prevent races. It can't be done in a central location. I'm really concerned about drivers having to implement this and not getting it correct; particularly when moving a device to the end of the list is cheap, and it shouldn't be a problem to do the move unconditionally after a driver has been matches, but before probe() is called. We may be able to keep watch to make sure that drivers using probe deferral are also reordering themselves, but that does nothing for the cases described above where the link order of the drivers determines probe order, not the dap_list order. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] drivercore: Add driver probe deferral mechanism
On Fri, Oct 14, 2011 at 11:33 AM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 14 Oct 2011, Grant Likely wrote: I don't think the second part needs to be quite so invasive. Certainly drivers that never defer probes shouldn't require anything to be moved. Think about that a minute. Consider a dpm_list of devices: abcDefGh Now assume that D has an implicit dependency on G. If D gets probed first, then it will be deferred until G gets probed and then D will get retried and moved to the end of the list resulting in: abcefGhD Everything is good now for the order that things need to be suspended in. Now lets assume that the drivers get linked into the kernel in a different order (or the modules get loaded in a different order) and G gets probed first, followed by D. No deferral occurred and so no reordering occurs, resulting in: abcDefGh (unchanged) But now suspend is broken because D depends on G, but G will be suspended before D. However D sometimes does defer probes. Therefore the device always needs to be moved, even though this particular probe wasn't deferred. Yes, that's part of my point. This looks and smells like a bug to me. In fact, even without using probe deferral it looks like a bug because the dap_list isn't taking into account the fact that there are very likely to be implicit dependencies between devices that are not represented in the device hierarchy (maybe not common in PCs, but certainly is the case for embedded). But, it is also easy to solve by ensuring the dap_list is also probe-order sorted. A deferred probe _should_ move the device to the end of the list. But this needs to happen in the probe routine itself (after it has verified that all the other required devices are present and before it has registered any children) to prevent races. It can't be done in a central location. I'm really concerned about drivers having to implement this and not getting it correct; particularly when moving a device to the end of the list is cheap, and it shouldn't be a problem to do the move unconditionally after a driver has been matches, but before probe() is called. But that's too early. What if D gets moved to the end of the list, then G gets added to the list and probed, and then D's probe succeeds? So the argument is that if really_probe() called dpm_move_last() immediately before the dev-bus-probe()/drv-probe() call then there is a race condition that G could get both registered and probed before D's probe() starts using G's resources. And so, the list would still have G after D which is in the wrong order. Do I understand correctly? And after the probe returns is too late, because by then there may well be child devices. Agreed, after probe returns is definitely too late. We may be able to keep watch to make sure that drivers using probe deferral are also reordering themselves, but that does nothing for the cases described above where the link order of the drivers determines probe order, not the dap_list order. Devices need to be moved whenever they have any external dependencies, regardless of the particular order they get probed in. I suspect this gets messy in a hurry, but perhaps it is worth trying. So, any device that makes use of a PHY, GPIO line, codec, etc will need to call dpm_move_last() before adding child devices, correct? I'd be much happier to find a way to do this in core code though. And there is still a potential race condition here. For example, if G is in the middle of it's probe routine, and D gets probed between G registering GPIOs and calling dpm_move_last(), then we're in the same boat again. I think the window for this race can be considered to be of the same magnitude as the moved to early race described above. I need to think about this more... g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] drivercore: Add driver probe deferral mechanism
On Thu, Oct 13, 2011 at 10:31:45AM -0400, Alan Stern wrote: On Thu, 13 Oct 2011, Ming Lei wrote: Inside device_add(), device_pm_add is called before bus_probe_device, so the patch can't change the device order in pm list, and just change the driver probe order. That's the way it works now, but can it be reworked? ?It would be IMO, it depends on what shape you plan to rework. Currently, the deferred probe may found a resource dependency, but I am not sure that pm dependency is same with the resource dependency found during probe. possible to adjust the list order after successful probe. ?However, I'm not clear on the ordering rules for the dpm_list. ?Right now it is explicitly ordered to have parents before children, but as already expressed, that doesn't accurately represent ordering constraints for multiple device dependancies. Maybe we should understand the correct model of the ordering constraints for the multiple device dependancies first, could you give a description or some examples about it? The requirement is that devices must be capable of resuming in the order given by dpm_list, and they must be capable of suspending in the reverse order. Therefore if device A can't work unless device B is functional, then B must come before A in dpm_list. For the deferred case; here is an example of the additional constraint. Consider the following hierarchy: -A +-B | +-C | +-D | +-E +-F +-G dpm_list could be ordered in at least the following ways (depending on exactly when devices get registered). There are many permutation, but children are always be listed after its direct parent. 1) ABECDFG (breadth first) 2) AEBFGCD (breadth first) 3) ABCDEFG (depth first) 4) AEFGBCD (depth first) Now, assume that device B depends on device F, and also assume that there is no way either to express that in the hierarchy or even for the constraint to be known at device registration time (the is exactly the situation we're dealing with on embedded platforms). Only the driver for B knows that it needs a resource provided by F's driver. So, the situation becomes that the ordering of dpm_list must now also be sorted so that non-tree dependencies are also accounted for. Of the list above, only sort order 4 meets the new constraint. The question then becomes, how can the dpm_list get resorted dynamically at runtime to ensure that the new constraints are always met without breaking old ones. Here are some options I can think of: 1) When a deferred probe succeeds, move the deferred device to the end of the dpm_list. Then the new sort order might be one of: 1) AECDFGB 2) AEFGCDB 3) ACDEFGB 4) AEFGCDB However, that approach breaks the guarantee that children appear after their parents (C D appear before B in all cases above) 2a) When a deferred probe succeeds, move the deferred device and it's entire child hierarchy to the end of the list to become one of: 1) AEFGBCD 2) AEFGBCD 3) AEFGBCD 4) AEFGBCD (hey! they're all the same now, but there are other orderings possible that aren't) :-) Problem: Complexity increases. This requires iterating through all the children and performing a reorder for each. Fortunately, this should be too expensive since I believe each individual move is an O(1) operation. I don't think the code will need to walk the list for each device. Potential problem: This may result in unnecessary sorting in a lot of cases. It may be that the newly probed device is already after the device it depends on, but the driver just hadn't been probed early enough to avoid deferral. Potential problem: It may end up exposing implicit dependencies between drivers that weren't observed before. Potential problem: This completely breaks if a parent gets probed *after* its child which might happen if something other than the parent driver creates the child devices. I don't think this is a real problem, but I think the kernel would first need to ensure that none of the children are bound to drivers, and complain loudly if they are. Otherwise the dpm_list won't reflect probe order. 2b) alternately, when *any* probe succeeds, move the deferred device and its children to the end of the list. This continues to maintain the parent-child relationship, and it ensures that the dpm_list is always also sorted in probe-order (devices bound to drivers will collect at the end of the list). Potential problem: On a big device hierarchy, this will mean a lot of moves. It should still only be O(1) for each move, but O(N^2) over probing the entire hierarchy. Devices will end up being moved once for itself, and once for each parent and grandparent bound to a driver. It could become slow. This option also has the potential problem when a parent gets probed after its child as discussed in 2a. 3) Add devices to dpm_list after successful probe instead of at
Re: [PATCH 2/5] drivercore: Add driver probe deferral mechanism
On Thu, Oct 13, 2011 at 02:16:42PM -0400, Alan Stern wrote: On Thu, 13 Oct 2011, Grant Likely wrote: For the deferred case; here is an example of the additional constraint. Consider the following hierarchy: -A +-B | +-C | +-D | +-E +-F +-G dpm_list could be ordered in at least the following ways (depending on exactly when devices get registered). There are many permutation, but children are always be listed after its direct parent. 1) ABECDFG (breadth first) 2) AEBFGCD (breadth first) 3) ABCDEFG (depth first) 4) AEFGBCD (depth first) Now, assume that device B depends on device F, and also assume that there is no way either to express that in the hierarchy or even for the constraint to be known at device registration time (the is exactly the situation we're dealing with on embedded platforms). Only the driver for B knows that it needs a resource provided by F's driver. So, the situation becomes that the ordering of dpm_list must now also be sorted so that non-tree dependencies are also accounted for. Of the list above, only sort order 4 meets the new constraint. The question then becomes, how can the dpm_list get resorted dynamically at runtime to ensure that the new constraints are always met without breaking old ones. Here are some options I can think of: This was a long message and I haven't absorbed the whole thing. heh; I did get rather verbose there. However it's worth pointing out right at the start that we already have device_pm_move_before(), device_pm_move_after(), and device_pm_move_last(). They are intended specifically to provide drivers with a way of making sure that dpm_list is in the right order -- people have been aware of these issues for some time. I saw those. I also notice that they are only used by device_move() when reparenting a device (which is another wrinkle I hadn't though about). Reparenting a device becomes problematic if the probe order is also represented in the list. If device_move() gets called, then any implicit probe-order sorting for that device would be lost. I also notice that device_move disregards any children when moving a device, which could also be a problem. Although it looks like the only users of device_move are: drivers/media/video/pvrusb2/pvrusb2-v4l2.c drivers/s390/cio/device.c net/bluetooth/hci_sysfs.c net/bluetooth/rfcomm/tty.c So it may not be significant to adapt. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] drivercore: Add driver probe deferral mechanism
On Tue, Oct 11, 2011 at 08:29:18PM +0800, Ming Lei wrote: On Tue, Oct 11, 2011 at 1:37 AM, Andrei Warkentin awarken...@vmware.com wrote: Hi, - Original Message - From: Greg KH g...@kroah.com To: Josh Triplett j...@joshtriplett.org Cc: G, Manjunath Kondaiah manj...@ti.com, linux-arm-ker...@lists.infradead.org, Grant Likely grant.lik...@secretlab.ca, linux-o...@vger.kernel.org, linux-mmc@vger.kernel.org, linux-ker...@vger.kernel.org, Dilan Lee di...@nvidia.com, Mark Brown broo...@opensource.wolfsonmicro.com, manjun...@jasper.es Sent: Saturday, October 8, 2011 11:55:02 AM Subject: Re: [PATCH 2/5] drivercore: Add driver probe deferral mechanism I'm a bit of a fly on the wall here, but I'm curious how this impacts suspend/resume. device_initialize-device_pm_init are called from device_register, so certainly this patch doesn't also ensure that the PM ordering matches probe ordering, which is bound to break suspend, right? Was this ever tested with the OMAP target? Shouldn't the Inside device_add(), device_pm_add is called before bus_probe_device, so the patch can't change the device order in pm list, and just change the driver probe order. That's the way it works now, but can it be reworked? It would be possible to adjust the list order after successful probe. However, I'm not clear on the ordering rules for the dpm_list. Right now it is explicitly ordered to have parents before children, but as already expressed, that doesn't accurately represent ordering constraints for multiple device dependancies. So, reordering the list would probably require maintaining the existing parent-child ordering constraint, but to also shift devices (and any possible children?) to be after drivers that are already probed. That alone will be difficult to implement and get right, but maybe the constraints can be simplified. It needs some further thought. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc 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/5] drivercore: add new error value for deferred probe
On Wed, Oct 12, 2011 at 11:48:23AM +0530, G, Manjunath Kondaiah wrote: On Sun, Oct 09, 2011 at 06:06:56PM -0700, Greg KH wrote: On Sun, Oct 09, 2011 at 04:59:31PM -0600, Grant Likely wrote: On Fri, Oct 7, 2011 at 6:12 PM, Greg KH g...@kroah.com wrote: On Fri, Oct 07, 2011 at 07:28:33PM -0400, valdis.kletni...@vt.edu wrote: On Fri, 07 Oct 2011 16:12:45 MDT, Grant Likely said: On Fri, Oct 7, 2011 at 12:43 AM, Greg KH g...@kroah.com wrote: On Fri, Oct 07, 2011 at 10:33:06AM +0500, G, Manjunath Kondaiah wrote: +#define EPROBE_DEFER 517 /* restart probe again after some time */ Can we really do this? According to Arnd, yes this is okay. Isn't this some user/kernel api here? What's wrong with just overloading on top of an existing error code? Surely one of the other 516 types could be used here, right? overloading makes it really hard to find the users at a later date. Would proposing '#define EPROBE_DEFER EAGAIN' be acceptable to everybody? That would allow overloading EAGAIN, but still make it easy to tell the usages apart if we need to separate them later... Yes, please do that, it is what USB does for it's internal error code handling. Really? When we've only currently used approximately 2^9 of a 2^31 numberspace? I'm fine with making sure that the number doesn't show up in the userspace headers, but it makes no sense to overload the #defines. Particularly so in this case where it isn't feasible to audit every driver to figure out what probe might possibly return. It is well within the realm of possibility that existing drivers are already returning -EAGAIN. I doubt they are, but you are right, it's really hard to tell. Besides; linux/errno.h *already* has linux-internal error codes that do not get exported out to userspace. There is an #ifdef __KERNEL__ block around ERESTARTSYS through EIOCBRETRY which is filtered out when exporting headers. I can't see any possible reason why we wouldn't add Linux internal error codes here. As long as it stays internal, that's fine, I was worried that this would be exported to userspace. Alan, still object to this? I hope no one has objections to use EPROBE_DEFER I say go with that value. If Alan still objects, then he will speak up. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] gpiolib: handle deferral probe error
On Wed, Oct 12, 2011 at 11:44:32AM +0530, G, Manjunath Kondaiah wrote: On Fri, Oct 07, 2011 at 04:09:38PM -0600, Grant Likely wrote: On Fri, Oct 7, 2011 at 4:06 AM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: On Fri, 07 Oct 2011 10:33:09 +0500 G, Manjunath Kondaiah manj...@ti.com wrote: The gpio library should return -EPROBE_DEFER in gpio_request if gpio driver is not ready. Why not use the perfectly good existing error codes we have for this ? We have EAGAIN and EUNATCH both of which look sensible. I want a distinct error code for probe deferral so that a) it doesn't overlap with something a driver is already doing, and b) so that all the users can be found again at a later date. That said, I'm not in agreement with this patch. It is fine for gpio lib to have a code that means the pin doesn't exist (yet), but the device driver needs to be the one to decide whether or not it is appropriate to use probe deferral. During gpio_request, driver gpio_request is not available. How can we expect driver to request deferred probe in this case? If gpio_request fails, the driver can then explicitly make the decision to return -EPROBE_DEFER. It isn't forced to pass on the error code from gpio_request(). g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc 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/5] drivercore: add new error value for deferred probe
On Fri, Oct 7, 2011 at 6:12 PM, Greg KH g...@kroah.com wrote: On Fri, Oct 07, 2011 at 07:28:33PM -0400, valdis.kletni...@vt.edu wrote: On Fri, 07 Oct 2011 16:12:45 MDT, Grant Likely said: On Fri, Oct 7, 2011 at 12:43 AM, Greg KH g...@kroah.com wrote: On Fri, Oct 07, 2011 at 10:33:06AM +0500, G, Manjunath Kondaiah wrote: +#define EPROBE_DEFER 517 /* restart probe again after some time */ Can we really do this? According to Arnd, yes this is okay. Isn't this some user/kernel api here? What's wrong with just overloading on top of an existing error code? Surely one of the other 516 types could be used here, right? overloading makes it really hard to find the users at a later date. Would proposing '#define EPROBE_DEFER EAGAIN' be acceptable to everybody? That would allow overloading EAGAIN, but still make it easy to tell the usages apart if we need to separate them later... Yes, please do that, it is what USB does for it's internal error code handling. Really? When we've only currently used approximately 2^9 of a 2^31 numberspace? I'm fine with making sure that the number doesn't show up in the userspace headers, but it makes no sense to overload the #defines. Particularly so in this case where it isn't feasible to audit every driver to figure out what probe might possibly return. It is well within the realm of possibility that existing drivers are already returning -EAGAIN. Besides; linux/errno.h *already* has linux-internal error codes that do not get exported out to userspace. There is an #ifdef __KERNEL__ block around ERESTARTSYS through EIOCBRETRY which is filtered out when exporting headers. I can't see any possible reason why we wouldn't add Linux internal error codes here. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] drivercore: Add driver probe deferral mechanism
On Fri, Oct 7, 2011 at 12:49 AM, Greg KH g...@kroah.com wrote: On Fri, Oct 07, 2011 at 10:33:07AM +0500, G, Manjunath Kondaiah wrote: +config PROBE_DEFER + bool Deferred Driver Probe + default y + help + This option provides deferring driver probe if it has dependency on + other driver. Without this feature, initcall ordering should be done + manually to resolve driver dependencies. This feature completely side + steps the issues by allowing driver registration to occur in any + order, and any driver can request to be retried after a few more other + drivers get probed. Why is this even an option? Why would you ever want it disabled? Why does it need to be selected? If you are going to default something to 'y' then just make it so it can't be turned off any other way by just not making it an option at all. It also cleans up this diff a lot, as you really don't want #ifdef in .c files. Okay, we'll drop the kconfig + * This bit is tricky. We want to process every device in the + * deferred list, but devices can be removed from the list at any + * time while inside this for-each loop. There are two things that + * need to be protected against: That's what the klist structure is supposed to make easier, why not use that here? + * - if the device is removed from the deferred_probe_list, then we + * loose our place in the loop. Since any device can be removed + * asynchronously, list_for_each_entry_safe() wouldn't make things + * much better. Simplest solution is to restart walking the list + * whenever the current device gets removed. Not the most efficient, + * but is simple to implement and easy to audit for correctness. + * - if the device is unregistered, and freed, then there is a risk + * of a null pointer dereference. This code uses get/put_device() + * to ensure the device cannot disappear from under our feet. Ick, use a klist, it was created to handle this exact problem in the driver core, so to not use it would be wrong, right? This comment block is stale. I reworked the code before I handed it off to Manjunath, but I never rewrote the comment. The current code uses a pair of list to keep deferred devices separate from devices currently being probed, and the problem described above no longer exists. However, Manjunath and I will look into switching from the two list design to using klist. Thanks for the feedback. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] gpiolib: handle deferral probe error
On Fri, Oct 7, 2011 at 4:06 AM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: On Fri, 07 Oct 2011 10:33:09 +0500 G, Manjunath Kondaiah manj...@ti.com wrote: The gpio library should return -EPROBE_DEFER in gpio_request if gpio driver is not ready. Why not use the perfectly good existing error codes we have for this ? We have EAGAIN and EUNATCH both of which look sensible. I want a distinct error code for probe deferral so that a) it doesn't overlap with something a driver is already doing, and b) so that all the users can be found again at a later date. That said, I'm not in agreement with this patch. It is fine for gpio lib to have a code that means the pin doesn't exist (yet), but the device driver needs to be the one to decide whether or not it is appropriate to use probe deferral. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc 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/5] drivercore: add new error value for deferred probe
On Fri, Oct 7, 2011 at 12:43 AM, Greg KH g...@kroah.com wrote: On Fri, Oct 07, 2011 at 10:33:06AM +0500, G, Manjunath Kondaiah wrote: Add new error value so that drivers can request deferred probe from drivercore. Signed-off-by: G, Manjunath Kondaiah manj...@ti.com Reported-by: Grant Likely grant.lik...@secretlab.ca --- Cc: linux-o...@vger.kernel.org Cc: linux-mmc@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: Grant Likely grant.lik...@secretlab.ca Cc: Greg Kroah-Hartman g...@kroah.com Cc: Dilan Lee di...@nvidia.com Cc: Mark Brown broo...@opensource.wolfsonmicro.com Cc: Manjunath GKondaiah manjunath.gkonda...@linaro.org Cc: Arnd Bergmann a...@arndb.de include/linux/errno.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/include/linux/errno.h b/include/linux/errno.h index 4668583..83d8fcf 100644 --- a/include/linux/errno.h +++ b/include/linux/errno.h @@ -16,6 +16,7 @@ #define ERESTARTNOHAND 514 /* restart if no handler.. */ #define ENOIOCTLCMD 515 /* No ioctl command */ #define ERESTART_RESTARTBLOCK 516 /* restart by calling sys_restart_syscall */ +#define EPROBE_DEFER 517 /* restart probe again after some time */ Can we really do this? According to Arnd, yes this is okay. Isn't this some user/kernel api here? What's wrong with just overloading on top of an existing error code? Surely one of the other 516 types could be used here, right? overloading makes it really hard to find the users at a later date. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] omap: hsmmc: use platform_driver_register
On Thu, Oct 6, 2011 at 11:05 PM, G, Manjunath Kondaiah manj...@ti.com wrote:
Existing omap hsmmc driver uses platform_driver_probe in init
function. Change it to use platform_driver_register in order to
use deferral probe mechanism.
Signed-off-by: G, Manjunath Kondaiah manj...@ti.com
Reported-by: Grant Likely grant.lik...@secretlab.ca
Acked-by: Grant Likely grant.lik...@secretlab.ca
---
Cc: linux-o...@vger.kernel.org
Cc: linux-mmc@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Greg Kroah-Hartman g...@kroah.com
Cc: Dilan Lee di...@nvidia.com
Cc: Mark Brown broo...@opensource.wolfsonmicro.com
Cc: Manjunath GKondaiah manjunath.gkonda...@linaro.org
Cc: Arnd Bergmann a...@arndb.de
drivers/mmc/host/omap_hsmmc.c | 7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 21e4a79..8dd2e7c 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1862,7 +1862,7 @@ static void omap_hsmmc_debugfs(struct mmc_host *mmc)
#endif
-static int __init omap_hsmmc_probe(struct platform_device *pdev)
+static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
{
struct omap_mmc_platform_data *pdata = pdev-dev.platform_data;
struct mmc_host *mmc;
@@ -2077,6 +2077,7 @@ static int __init omap_hsmmc_probe(struct
platform_device *pdev)
pm_runtime_mark_last_busy(host-dev);
pm_runtime_put_autosuspend(host-dev);
+ dev_dbg(mmc_dev(host-mmc), Probe success...\n);
return 0;
err_slot_name:
@@ -2270,6 +2271,7 @@ static struct dev_pm_ops omap_hsmmc_dev_pm_ops = {
};
static struct platform_driver omap_hsmmc_driver = {
+ .probe = omap_hsmmc_probe,
.remove = omap_hsmmc_remove,
.driver = {
.name = DRIVER_NAME,
@@ -2280,8 +2282,7 @@ static struct platform_driver omap_hsmmc_driver = {
static int __init omap_hsmmc_init(void)
{
- /* Register the MMC driver */
- return platform_driver_probe(omap_hsmmc_driver, omap_hsmmc_probe);
+ return platform_driver_register(omap_hsmmc_driver);
}
static void __exit omap_hsmmc_cleanup(void)
--
1.7.4.1
--
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/
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: sdhci-tegra: Add #ifdef CONFIG_OF guard for of_find_property
On Mon, Oct 03, 2011 at 08:59:45AM -0700, Stephen Warren wrote: Igor Grinberg wrote at Monday, October 03, 2011 2:32 AM: Hi Axel, On 10/03/11 06:07, Axel Lin wrote: I got below build error with make tegra_defconfig;make Add #ifdef CONFIG_OF guard for of_find_property to fix below build error: CC drivers/mmc/host/sdhci-tegra.o drivers/mmc/host/sdhci-tegra.c: In function 'sdhci_tegra_dt_parse_pdata': drivers/mmc/host/sdhci-tegra.c:157: error: implicit declaration of function 'of_find_property' ... diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c ... @@ -154,8 +154,11 @@ static struct tegra_sdhci_platform_data * __devinit sdhci_tegra_dt_parse_pdata( plat-cd_gpio = of_get_named_gpio(np, cd-gpios, 0); plat-wp_gpio = of_get_named_gpio(np, wp-gpios, 0); plat-power_gpio = of_get_named_gpio(np, power-gpios, 0); + +#ifdef CONFIG_OF if (of_find_property(np, support-8bit, NULL)) plat-is_8bit = 1; +#endif Shouldn't we add a stub for the of_find_property() method to include/linux/of.h instead of adding more ifdefs? Or may be use of_get_property() method instead? I submitted a patch to add a stub of_find_property a little while back. Per https://lkml.org/lkml/2011/9/22/301, Grant applied it already. I'm not sure why it hasn't shown up in linux-next, since Grant's repo wasn't affected by the kernel.org downtime. Sorry for the breakage. That's because I forgot to push out the tree. :-( Sorry. I've just fixed it now. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc 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/3] arm/tegra: Move EN_VDD_1V05_GPIO to board-harmony.h
On Thu, Sep 22, 2011 at 05:32:58PM +0100, Russell King - ARM Linux wrote: On Thu, Sep 22, 2011 at 09:31:48AM -0700, Stephen Warren wrote: Russell King - ARM Linux wrote at Thursday, September 22, 2011 2:12 AM: On Wed, Sep 21, 2011 at 01:16:49PM -0700, Olof Johansson wrote: Works for me. Or I could stage it in a topic branch that would be merged after Russell's GPIO tree. Holding stuff off from being merged doesn't work. Russell, So I take it I should add the two ack'd patches to the ARM patch system, and have you pick them up? That's a workable solution, yes. I've done that with various other gpio patches from people, and Grant seems happy with that. Yes, I'm comfortable with patches going in via other trees when there are dependencies. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] arm/tegra: Replace mach/gpio.h with mach/gpio-tegra.h
On Wed, Sep 21, 2011 at 01:33:39PM -0600, Stephen Warren wrote:
This will eventually allow mach/gpio.h to be deleted. This mirrors
LinusW's recent equivalent work on various other ARM platforms.
Signed-off-by: Stephen Warren swar...@nvidia.com
Acked-by: Grant Likely grant.lik...@secretlab.ca
---
arch/arm/mach-tegra/board-harmony.h |2 +
arch/arm/mach-tegra/board-paz00.h |2 +
arch/arm/mach-tegra/board-seaboard.h |2 +
arch/arm/mach-tegra/board-trimslice.h |2 +
arch/arm/mach-tegra/include/mach/gpio-tegra.h | 39
+
arch/arm/mach-tegra/include/mach/gpio.h | 39
-
arch/arm/mach-tegra/usb_phy.c |1 +
drivers/gpio/gpio-tegra.c |1 +
drivers/mmc/host/sdhci-tegra.c|2 +
9 files changed, 51 insertions(+), 39 deletions(-)
create mode 100644 arch/arm/mach-tegra/include/mach/gpio-tegra.h
diff --git a/arch/arm/mach-tegra/board-harmony.h
b/arch/arm/mach-tegra/board-harmony.h
index 280d203..139d96c 100644
--- a/arch/arm/mach-tegra/board-harmony.h
+++ b/arch/arm/mach-tegra/board-harmony.h
@@ -17,6 +17,8 @@
#ifndef _MACH_TEGRA_BOARD_HARMONY_H
#define _MACH_TEGRA_BOARD_HARMONY_H
+#include mach/gpio-tegra.h
+
#define HARMONY_GPIO_TPS6586X(_x_) (TEGRA_NR_GPIOS + (_x_))
#define HARMONY_GPIO_WM8903(_x_) (HARMONY_GPIO_TPS6586X(4) + (_x_))
diff --git a/arch/arm/mach-tegra/board-paz00.h
b/arch/arm/mach-tegra/board-paz00.h
index 86057c3..2dc1899 100644
--- a/arch/arm/mach-tegra/board-paz00.h
+++ b/arch/arm/mach-tegra/board-paz00.h
@@ -17,6 +17,8 @@
#ifndef _MACH_TEGRA_BOARD_PAZ00_H
#define _MACH_TEGRA_BOARD_PAZ00_H
+#include mach/gpio-tegra.h
+
/* SDCARD */
#define TEGRA_GPIO_SD1_CDTEGRA_GPIO_PV5
#define TEGRA_GPIO_SD1_WPTEGRA_GPIO_PH1
diff --git a/arch/arm/mach-tegra/board-seaboard.h
b/arch/arm/mach-tegra/board-seaboard.h
index d06c334..4c45d4c 100644
--- a/arch/arm/mach-tegra/board-seaboard.h
+++ b/arch/arm/mach-tegra/board-seaboard.h
@@ -17,6 +17,8 @@
#ifndef _MACH_TEGRA_BOARD_SEABOARD_H
#define _MACH_TEGRA_BOARD_SEABOARD_H
+#include mach/gpio-tegra.h
+
#define SEABOARD_GPIO_TPS6586X(_x_) (TEGRA_NR_GPIOS + (_x_))
#define SEABOARD_GPIO_WM8903(_x_)(SEABOARD_GPIO_TPS6586X(4) + (_x_))
diff --git a/arch/arm/mach-tegra/board-trimslice.h
b/arch/arm/mach-tegra/board-trimslice.h
index 7a7dee8..50f128d 100644
--- a/arch/arm/mach-tegra/board-trimslice.h
+++ b/arch/arm/mach-tegra/board-trimslice.h
@@ -17,6 +17,8 @@
#ifndef _MACH_TEGRA_BOARD_TRIMSLICE_H
#define _MACH_TEGRA_BOARD_TRIMSLICE_H
+#include mach/gpio-tegra.h
+
#define TRIMSLICE_GPIO_SD4_CDTEGRA_GPIO_PP1 /* mmc4 cd */
#define TRIMSLICE_GPIO_SD4_WPTEGRA_GPIO_PP2 /* mmc4 wp */
diff --git a/arch/arm/mach-tegra/include/mach/gpio-tegra.h
b/arch/arm/mach-tegra/include/mach/gpio-tegra.h
new file mode 100644
index 000..87d37fd
--- /dev/null
+++ b/arch/arm/mach-tegra/include/mach/gpio-tegra.h
@@ -0,0 +1,39 @@
+/*
+ * arch/arm/mach-tegra/include/mach/gpio.h
+ *
+ * Copyright (C) 2010 Google, Inc.
+ *
+ * Author:
+ * Erik Gilling konk...@google.com
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+ */
+
+#ifndef __MACH_TEGRA_GPIO_TEGRA_H
+#define __MACH_TEGRA_GPIO_TEGRA_H
+
+#include linux/types.h
+#include mach/irqs.h
+
+#define TEGRA_NR_GPIOS INT_GPIO_NR
+
+#define TEGRA_GPIO_TO_IRQ(gpio) (INT_GPIO_BASE + (gpio))
+
+struct tegra_gpio_table {
+ int gpio; /* GPIO number */
+ boolenable; /* Enable for GPIO at init? */
+};
+
+void tegra_gpio_config(struct tegra_gpio_table *table, int num);
+void tegra_gpio_enable(int gpio);
+void tegra_gpio_disable(int gpio);
+
+#endif
diff --git a/arch/arm/mach-tegra/include/mach/gpio.h
b/arch/arm/mach-tegra/include/mach/gpio.h
index 7910d26..e69de29 100644
--- a/arch/arm/mach-tegra/include/mach/gpio.h
+++ b/arch/arm/mach-tegra/include/mach/gpio.h
@@ -1,39 +0,0 @@
-/*
- * arch/arm/mach-tegra/include/mach/gpio.h
- *
- * Copyright (C) 2010 Google, Inc.
- *
- * Author:
- * Erik Gilling konk...@google.com
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY
Re: [PATCH REPOST 1/2] arm/dt: Tegra: Update SDHCI nodes to match bindings
On Tue, Sep 20, 2011 at 10:46:25AM -0600, Stephen Warren wrote:
The bindings were recently updated to have separate properties for each
type of GPIO. Update the Device Tree source to match that.
Signed-off-by: Stephen Warren swar...@nvidia.com
Acked-by: Olof Johansson o...@lixom.net
Acked-by: Grant Likely grant.lik...@secretlab.ca
---
I'd previously sent these to Grant assuming they'd go in his dt/next branch,
but perhaps these should go in through Arnd's arm-soc next/dt branch?
Yes, they should probably go via Arnd's tree.
g.
arch/arm/boot/dts/tegra-harmony.dts | 12 ++--
arch/arm/boot/dts/tegra-seaboard.dts |6 +++---
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/arm/boot/dts/tegra-harmony.dts
b/arch/arm/boot/dts/tegra-harmony.dts
index 4c05334..e581866 100644
--- a/arch/arm/boot/dts/tegra-harmony.dts
+++ b/arch/arm/boot/dts/tegra-harmony.dts
@@ -57,14 +57,14 @@
};
sdhci@c8000200 {
- gpios = gpio 69 0, /* cd, gpio PI5 */
- gpio 57 0, /* wp, gpio PH1 */
- gpio 155 0; /* power, gpio PT3 */
+ cd-gpios = gpio 69 0; /* gpio PI5 */
+ wp-gpios = gpio 57 0; /* gpio PH1 */
+ power-gpios = gpio 155 0; /* gpio PT3 */
};
sdhci@c8000600 {
- gpios = gpio 58 0, /* cd, gpio PH2 */
- gpio 59 0, /* wp, gpio PH3 */
- gpio 70 0; /* power, gpio PI6 */
+ cd-gpios = gpio 58 0; /* gpio PH2 */
+ wp-gpios = gpio 59 0; /* gpio PH3 */
+ power-gpios = gpio 70 0; /* gpio PI6 */
};
};
diff --git a/arch/arm/boot/dts/tegra-seaboard.dts
b/arch/arm/boot/dts/tegra-seaboard.dts
index 1940cae..64cedca 100644
--- a/arch/arm/boot/dts/tegra-seaboard.dts
+++ b/arch/arm/boot/dts/tegra-seaboard.dts
@@ -21,8 +21,8 @@
};
sdhci@c8000400 {
- gpios = gpio 69 0, /* cd, gpio PI5 */
- gpio 57 0, /* wp, gpio PH1 */
- gpio 70 0; /* power, gpio PI6 */
+ cd-gpios = gpio 69 0; /* gpio PI5 */
+ wp-gpios = gpio 57 0; /* gpio PH1 */
+ power-gpios = gpio 70 0; /* gpio PI6 */
};
};
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH REPOST 2/2] arm/dt: Tegra: Add support-8bit to SDHCI nodes
On Tue, Sep 20, 2011 at 10:46:26AM -0600, Stephen Warren wrote:
For Seaboard's internal eMMC, this makes the difference between a
5.5MB/s and 10.2MB/s transfer rate. On Harmony, there wasn't any
measurable difference on my cheap/slow ~2MB/s card.
Signed-off-by: Stephen Warren swar...@nvidia.com
Acked-by: Grant Likely grant.lik...@secretlab.ca
---
arch/arm/boot/dts/tegra-harmony.dts |1 +
arch/arm/boot/dts/tegra-seaboard.dts |4
2 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/tegra-harmony.dts
b/arch/arm/boot/dts/tegra-harmony.dts
index e581866..0e225b8 100644
--- a/arch/arm/boot/dts/tegra-harmony.dts
+++ b/arch/arm/boot/dts/tegra-harmony.dts
@@ -66,5 +66,6 @@
cd-gpios = gpio 58 0; /* gpio PH2 */
wp-gpios = gpio 59 0; /* gpio PH3 */
power-gpios = gpio 70 0; /* gpio PI6 */
+ support-8bit;
};
};
diff --git a/arch/arm/boot/dts/tegra-seaboard.dts
b/arch/arm/boot/dts/tegra-seaboard.dts
index 64cedca..a72299b 100644
--- a/arch/arm/boot/dts/tegra-seaboard.dts
+++ b/arch/arm/boot/dts/tegra-seaboard.dts
@@ -25,4 +25,8 @@
wp-gpios = gpio 57 0; /* gpio PH1 */
power-gpios = gpio 70 0; /* gpio PI6 */
};
+
+ sdhci@c8000600 {
+ support-8bit;
+ };
};
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH REPOST 1/2] arm/dt: Tegra: Update SDHCI nodes to match bindings
On Tue, Sep 20, 2011 at 07:43:29PM +0200, Arnd Bergmann wrote: On Tuesday 20 September 2011, Stephen Warren wrote: The bindings were recently updated to have separate properties for each type of GPIO. Update the Device Tree source to match that. Signed-off-by: Stephen Warren swar...@nvidia.com Acked-by: Olof Johansson o...@lixom.net --- I'd previously sent these to Grant assuming they'd go in his dt/next branch, but perhaps these should go in through Arnd's arm-soc next/dt branch? Which tree has the update that changed the bindings? I think it should go into the same one. If it's already upstream, I can take it into the fixes branch. Already upstream g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc 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/3] mmc: sdhci-s3c: Add support for device tree based probe
On Fri, Jul 15, 2011 at 05:12:06PM +0530, Thomas Abraham wrote:
Add of_match_table to enable sdhci-s3c driver to be probed when a compatible
sdhci device node is found in device tree.
CC: linux-mmc@vger.kernel.org
Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
---
.../devicetree/bindings/mmc/samsung-s3c-sdhci.txt | 10 ++
drivers/mmc/host/sdhci-s3c.c | 11 +++
2 files changed, 21 insertions(+), 0 deletions(-)
create mode 100644
Documentation/devicetree/bindings/mmc/samsung-s3c-sdhci.txt
diff --git a/Documentation/devicetree/bindings/mmc/samsung-s3c-sdhci.txt
b/Documentation/devicetree/bindings/mmc/samsung-s3c-sdhci.txt
new file mode 100644
index 000..c2298f8
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/samsung-s3c-sdhci.txt
@@ -0,0 +1,10 @@
+* Samsung's SDHCI controller
+
+The Samsung's SDHCI controller is used for interfacing with SD/MMC cards.
+
+Required properties:
+- compatible : should be samsung,s3c-sdhci
+- reg : base physical address of the controller and length of memory mapped
+ region.
+- interrupts : interrupt number to the cpu.
+
diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 69e3ee3..5ccbee0 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -629,6 +629,16 @@ static int sdhci_s3c_resume(struct platform_device *dev)
#define sdhci_s3c_resume NULL
#endif
+#ifdef CONFIG_OF
+static const struct of_device_id s3c_sdhci_match[] = {
+ { .compatible = samsung,s3c-sdhci },
Be specific. samsung,exynos4210-sdhci. Newer chips can claim
compatibility with the older one.
Otherwise,
Acked-by: Grant Likely grant.lik...@secretlab.ca
+ {},
+};
+MODULE_DEVICE_TABLE(of, s3c_sdhci_match);
+#else
+#define s3c_sdhci_match NULL
+#endif
+
static struct platform_driver sdhci_s3c_driver = {
.probe = sdhci_s3c_probe,
.remove = __devexit_p(sdhci_s3c_remove),
@@ -637,6 +647,7 @@ static struct platform_driver sdhci_s3c_driver = {
.driver = {
.owner = THIS_MODULE,
.name = s3c-sdhci,
+ .of_match_table = s3c_sdhci_match,
},
};
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support
On Thu, Jul 14, 2011 at 12:52 AM, Anton Vorontsov
cbouatmai...@gmail.com wrote:
Hi,
On Wed, Jul 06, 2011 at 03:05:18PM -0600, Grant Likely wrote:
On Thu, Jul 07, 2011 at 12:47:50AM +0800, Shawn Guo wrote:
The patch adds device tree probe support for sdhci-esdhc-imx driver.
Signed-off-by: Shawn Guo shawn@linaro.org
Cc: Wolfram Sang w.s...@pengutronix.de
Cc: Chris Ball c...@laptop.org
Cc: Grant Likely grant.lik...@secretlab.ca
Acked-by: Grant Likely grant.lik...@secretlab.ca
[...]
+Optional properties:
+- fsl,card-wired : Indicate the card is wired to host permanently
+- fsl,cd-internal : Indicate to use controller internal card detection
+- fsl,wp-internal : Indicate to use controller internal write protection
+- cd-gpios : Specify GPIOs for card detection
+- wp-gpios : Specify GPIOs for write protection
[...]
+ cd-gpios = gpio0 6 0; /* GPIO1_6 */
+ wp-gpios = gpio0 5 0; /* GPIO1_5 */
Is there any particular reason why we started using named GPIOs
in the device tree? To me, that's quite drastic change... should
we start using named regs and interrupts as well? Personally, I
don't think that the idea is great, as now you don't know where
to expect memory resources, interrupt resources and, eventually
GPIO resources.
No, there are no plans to move to named irqs or regs. irq and reg
resources tend to be a lot more regular than gpios. It's not a
drastic change though because existing bindings remain unaffected, and
new bindings can still use unnamed gpios if that is more appropriate.
Just a few disadvantages off the top of my head:
- You can't statically validate your device tree for correctness.
Today dtc checks #{address,size}-cells sanity for 'regs' and
'ranges';
We can. The gpio binding was extended to be in the form
[name-]gpios. Any property with the string gpios at the end can
be statically validated.
- You can't automatically convert named resources into 'struct
resource *', as we do for platform devices now;
Only for irqs and regs. gpios have never been automatically loaded
into resources.
- Any pros for using named resources in the device tree? I don't
see any.
Human readability. To know exactly what a gpio is intended to be used
for. Particularly for the case where a device might not use all the
gpios that it could use. Yes, the gpios property can have 'holes' in
it, but the observation was made by several people that it is easy to
get wrong. I for one thing the concern was well justified.
So, I suggest to at least discuss this stuff a little bit more
before polluting device trees with dubious ideas.
It was discussed on list quite a while ago.
p.s.
As for this particular patch, I really don't see why we should
use named GPIOs. For consistency, I'd suggest to reuse bindings
from Documentation/devicetree/bindings/mmc/mmc-spi-slot.txt.
Plus, fsl,cd-internal and fsl,wp-internal is not needed: either
you specify GPIOs or not. That already signals whether driver
should use internal detection (i.e. 'present' register) or
external (i.e. GPIO).
wp-internal and cd-internal differentiates between using the internal
functionality and not having wp/cd at all.
And also, why {cd,wp}-gpioS (plural)?
For consistency. Doing it that way means that the plural gpios is
always the suffix for both the gpios and name-gpios use cases.
--
Anton Vorontsov
Email: cbouatmai...@gmail.com
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc 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 3/3] mmc: sdhci-esdhc-imx: add device tree probe support
On Wed, Jul 06, 2011 at 11:43:15PM +0800, Shawn Guo wrote: On Tue, Jul 05, 2011 at 11:54:34AM -0600, Grant Likely wrote: On Tue, Jul 5, 2011 at 9:26 AM, Shawn Guo shawn@linaro.org wrote: The patch adds device tree probe support for sdhci-esdhc-imx driver. Signed-off-by: Shawn Guo shawn@linaro.org Cc: Wolfram Sang w.s...@pengutronix.de Cc: Chris Ball c...@laptop.org Cc: Grant Likely grant.lik...@secretlab.ca --- .../devicetree/bindings/mmc/fsl-imx-esdhc.txt | 40 drivers/mmc/host/sdhci-esdhc-imx.c | 102 +++- 2 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt new file mode 100644 index 000..351d239 --- /dev/null +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt @@ -0,0 +1,40 @@ +* Freescale Enhanced Secure Digital Host Controller (eSDHC) for i.MX + +The Enhanced Secure Digital Host Controller on Freescale i.MX family +provides an interface for MMC, SD, and SDIO types of memory cards. + +Required properties: +- compatible : Should be fsl,chip-esdhc +- reg : Should contain eSDHC registers location and length +- interrupts : Should contain eSDHC interrupt +- cd-type : String, card detection (CD) method. Supported values are: + none : No CD + controller : Uses eSDHC controller internal CD signal + gpio : Uses GPIO pin for CD + permanent : No CD because card is permanently wired to host +- wp-type : String, write protection (WP) method. Supported values are: + none : No WP + controller : Uses eSDHC controller internal WP signal + gpio : Uses GPIO pin for WP +- gpios : Should specify GPIOs in this order: CD GPIO, WP GPIO, if + properties cd-type and wp-type are gpio. Again, be explicit in your gpios property names. Create a different property for each gpio: cd-gpios and wp-gpios. As for wp-type and cd-type, I think you can drop them. Default to internal controller CD and WP pins. Use gpio if cd-gpios or wp-gpios is present, and define specific properties for the no-wp, no-cd and fixed-card cases. (can you tell that I'm not a fan of the *-type binding for this driver?) :-) I would let default be no CD/WP, and define properties for controller internal CD/WP and wired case, if you do not see a problem with it. Okay. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/4] mmc: sdhci-esdhc-imx: do not reference platform data after probe
On Thu, Jul 07, 2011 at 12:47:47AM +0800, Shawn Guo wrote:
The patch copies platform data into pltfm_imx_data and reference
the data there than platform data after probe.
This work is inspired by Grant Likely and Troy Kisky.
Signed-off-by: Shawn Guo shawn@linaro.org
Cc: Troy Kisky troy.ki...@boundarydevices.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Wolfram Sang w.s...@pengutronix.de
Cc: Chris Ball c...@laptop.org
Acked-by: Grant Likely grant.lik...@secretlab.ca
---
drivers/mmc/host/sdhci-esdhc-imx.c | 22 +-
1 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index 3cc6f61..0b0e62e 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -45,6 +45,7 @@
struct pltfm_imx_data {
int flags;
u32 scratchpad;
+ struct esdhc_platform_data boarddata;
};
static unsigned int esdhc_pltfm_get_max_blk_size(struct sdhci_host *host)
@@ -69,8 +70,9 @@ static inline void esdhc_clrset_le(struct sdhci_host *host,
u32 mask, u32 val, i
static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
{
- struct esdhc_platform_data *boarddata =
- host-mmc-parent-platform_data;
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct pltfm_imx_data *imx_data = pltfm_host-priv;
+ struct esdhc_platform_data *boarddata = imx_data-boarddata;
/* fake CARD_PRESENT flag */
u32 val = readl(host-ioaddr + reg);
@@ -92,8 +94,7 @@ static void esdhc_writel_le(struct sdhci_host *host, u32
val, int reg)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct pltfm_imx_data *imx_data = pltfm_host-priv;
- struct esdhc_platform_data *boarddata =
- host-mmc-parent-platform_data;
+ struct esdhc_platform_data *boarddata = imx_data-boarddata;
if (unlikely((reg == SDHCI_INT_ENABLE || reg == SDHCI_SIGNAL_ENABLE)
(boarddata-cd_type == ESDHC_CD_GPIO)))
@@ -210,8 +211,9 @@ static unsigned int esdhc_pltfm_get_min_clock(struct
sdhci_host *host)
static unsigned int esdhc_pltfm_get_ro(struct sdhci_host *host)
{
- struct esdhc_platform_data *boarddata =
- host-mmc-parent-platform_data;
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct pltfm_imx_data *imx_data = pltfm_host-priv;
+ struct esdhc_platform_data *boarddata = imx_data-boarddata;
switch (boarddata-wp_type) {
case ESDHC_WP_GPIO:
@@ -291,12 +293,14 @@ static int __devinit sdhci_esdhc_imx_probe(struct
platform_device *pdev)
if (!(cpu_is_mx25() || cpu_is_mx35() || cpu_is_mx51()))
imx_data-flags |= ESDHC_FLAG_MULTIBLK_NO_INT;
- boarddata = host-mmc-parent-platform_data;
- if (!boarddata) {
+ if (!host-mmc-parent-platform_data) {
dev_err(mmc_dev(host-mmc), no board data!\n);
err = -EINVAL;
goto no_board_data;
}
+ imx_data-boarddata = *((struct esdhc_platform_data *)
+ host-mmc-parent-platform_data);
+ boarddata = imx_data-boarddata;
/* write_protect */
if (boarddata-wp_type == ESDHC_WP_GPIO) {
@@ -373,8 +377,8 @@ static int __devexit sdhci_esdhc_imx_remove(struct
platform_device *pdev)
{
struct sdhci_host *host = platform_get_drvdata(pdev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
- struct esdhc_platform_data *boarddata =
host-mmc-parent-platform_data;
struct pltfm_imx_data *imx_data = pltfm_host-priv;
+ struct esdhc_platform_data *boarddata = imx_data-boarddata;
int dead = (readl(host-ioaddr + SDHCI_INT_STATUS) == 0x);
sdhci_remove_host(host, dead);
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support
On Thu, Jul 07, 2011 at 12:47:50AM +0800, Shawn Guo wrote:
The patch adds device tree probe support for sdhci-esdhc-imx driver.
Signed-off-by: Shawn Guo shawn@linaro.org
Cc: Wolfram Sang w.s...@pengutronix.de
Cc: Chris Ball c...@laptop.org
Cc: Grant Likely grant.lik...@secretlab.ca
Acked-by: Grant Likely grant.lik...@secretlab.ca
---
.../devicetree/bindings/mmc/fsl-imx-esdhc.txt | 34 +
drivers/mmc/host/sdhci-esdhc-imx.c | 77
+---
2 files changed, 102 insertions(+), 9 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
new file mode 100644
index 000..ab22fe6
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
@@ -0,0 +1,34 @@
+* Freescale Enhanced Secure Digital Host Controller (eSDHC) for i.MX
+
+The Enhanced Secure Digital Host Controller on Freescale i.MX family
+provides an interface for MMC, SD, and SDIO types of memory cards.
+
+Required properties:
+- compatible : Should be fsl,chip-esdhc
+- reg : Should contain eSDHC registers location and length
+- interrupts : Should contain eSDHC interrupt
+
+Optional properties:
+- fsl,card-wired : Indicate the card is wired to host permanently
+- fsl,cd-internal : Indicate to use controller internal card detection
+- fsl,wp-internal : Indicate to use controller internal write protection
+- cd-gpios : Specify GPIOs for card detection
+- wp-gpios : Specify GPIOs for write protection
+
+Examples:
+
+esdhc@70004000 {
+ compatible = fsl,imx51-esdhc;
+ reg = 0x70004000 0x4000;
+ interrupts = 1;
+ fsl,cd-internal;
+ fsl,wp-internal;
+};
+
+esdhc@70008000 {
+ compatible = fsl,imx51-esdhc;
+ reg = 0x70008000 0x4000;
+ interrupts = 2;
+ cd-gpios = gpio0 6 0; /* GPIO1_6 */
+ wp-gpios = gpio0 5 0; /* GPIO1_5 */
+};
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index f3efb3b..cbdd91f 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -20,6 +20,9 @@
#include linux/mmc/host.h
#include linux/mmc/mmc.h
#include linux/mmc/sdio.h
+#include linux/of.h
+#include linux/of_device.h
+#include linux/of_gpio.h
#include mach/esdhc.h
#include sdhci-pltfm.h
#include sdhci-esdhc.h
@@ -73,6 +76,14 @@ static struct platform_device_id imx_esdhc_devtype[] = {
}
};
+static const struct of_device_id imx_esdhc_dt_ids[] = {
+ { .compatible = fsl,imx25-esdhc, .data =
imx_esdhc_devtype[IMX25_ESDHC], },
+ { .compatible = fsl,imx35-esdhc, .data =
imx_esdhc_devtype[IMX35_ESDHC], },
+ { .compatible = fsl,imx51-esdhc, .data =
imx_esdhc_devtype[IMX51_ESDHC], },
+ { .compatible = fsl,imx53-esdhc, .data =
imx_esdhc_devtype[IMX53_ESDHC], },
+ { /* sentinel */ }
+};
+
static inline int is_imx25_esdhc(struct pltfm_imx_data *data)
{
return data-devtype == IMX25_ESDHC;
@@ -307,8 +318,48 @@ static irqreturn_t cd_irq(int irq, void *data)
return IRQ_HANDLED;
};
+#ifdef CONFIG_OF
+static int __devinit
+sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
+ struct esdhc_platform_data *boarddata)
+{
+ struct device_node *np = pdev-dev.of_node;
+
+ if (!np)
+ return -ENODEV;
+
+ if (of_get_property(np, fsl,card-wired, NULL))
+ boarddata-cd_type = ESDHC_CD_PERMANENT;
+
+ if (of_get_property(np, fsl,cd-controller, NULL))
+ boarddata-cd_type = ESDHC_CD_CONTROLLER;
+
+ if (of_get_property(np, fsl,wp-controller, NULL))
+ boarddata-wp_type = ESDHC_WP_CONTROLLER;
+
+ boarddata-cd_gpio = of_get_named_gpio(np, cd-gpios, 0);
+ if (gpio_is_valid(boarddata-cd_gpio))
+ boarddata-cd_type = ESDHC_CD_GPIO;
+
+ boarddata-wp_gpio = of_get_named_gpio(np, wp-gpios, 0);
+ if (gpio_is_valid(boarddata-wp_gpio))
+ boarddata-wp_type = ESDHC_WP_GPIO;
+
+ return 0;
+}
+#else
+static inline int
+sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
+ struct esdhc_platform_data *boarddata)
+{
+ return -ENODEV;
+}
+#endif
+
static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
{
+ const struct of_device_id *of_id =
+ of_match_device(imx_esdhc_dt_ids, pdev-dev);
struct sdhci_pltfm_host *pltfm_host;
struct sdhci_host *host;
struct esdhc_platform_data *boarddata;
@@ -323,9 +374,13 @@ static int __devinit sdhci_esdhc_imx_probe(struct
platform_device *pdev)
pltfm_host = sdhci_priv(host);
imx_data = kzalloc(sizeof(struct pltfm_imx_data), GFP_KERNEL);
- if (!imx_data)
- return -ENOMEM;
+ if (!imx_data) {
+ err = -ENOMEM
Re: [PATCH 3/3] mmc: sdhci-esdhc-imx: add device tree probe support
On Tue, Jul 5, 2011 at 9:35 AM, Shawn Guo shawn@freescale.com wrote: On Mon, Jul 04, 2011 at 12:25:48AM -0600, Grant Likely wrote: + none : No CD + controller : Uses eSDHC controller internal CD signal + gpio : Uses GPIO pin for CD I would say the presence of a cd-gpios property would implicitly mean gpio is to be used for the CD pin. Yes, you are right. But I would say this is a direct translation of the existing platform_data. After all, we are sharing most of code path between platform and dt. Meh, that's just implementation detail and the DT bindings should *not* be focused on what Linux happens to currently do. Define a binding that makes sense first and foremost, and then make the driver use it. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc 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/3] mmc: sdhci-esdhc-imx: get rid of the uses of cpu_is_mx()
On Tue, Jul 5, 2011 at 9:26 AM, Shawn Guo shawn@linaro.org wrote:
The patch removes all the uses of cpu_is_mx(). Instead, it utilizes
platform_device_id to distinguish the esdhc differences among SoCs.
Signed-off-by: Shawn Guo shawn@linaro.org
Cc: Wolfram Sang w.s...@pengutronix.de
Cc: Chris Ball c...@laptop.org
Acked-by: Grant Likely grant.lik...@secretlab.ca
---
arch/arm/mach-imx/clock-imx25.c | 4 +-
arch/arm/mach-imx/clock-imx35.c | 6 +-
arch/arm/mach-mx5/clock-mx51-mx53.c | 16 +++---
arch/arm/mach-mx5/mx51_efika.c | 4 +-
.../plat-mxc/devices/platform-sdhci-esdhc-imx.c | 17 +++---
arch/arm/plat-mxc/include/mach/devices-common.h | 1 +
drivers/mmc/host/sdhci-esdhc-imx.c | 60 ++-
7 files changed, 81 insertions(+), 27 deletions(-)
diff --git a/arch/arm/mach-imx/clock-imx25.c b/arch/arm/mach-imx/clock-imx25.c
index a65838f..2955afa 100644
--- a/arch/arm/mach-imx/clock-imx25.c
+++ b/arch/arm/mach-imx/clock-imx25.c
@@ -300,8 +300,8 @@ static struct clk_lookup lookups[] = {
_REGISTER_CLOCK(imx2-wdt.0, NULL, wdt_clk)
_REGISTER_CLOCK(imx-ssi.0, NULL, ssi1_clk)
_REGISTER_CLOCK(imx-ssi.1, NULL, ssi2_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.0, NULL, esdhc1_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.1, NULL, esdhc2_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx25.0, NULL, esdhc1_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx25.1, NULL, esdhc2_clk)
_REGISTER_CLOCK(mx2-camera.0, NULL, csi_clk)
_REGISTER_CLOCK(NULL, audmux, audmux_clk)
_REGISTER_CLOCK(flexcan.0, NULL, can1_clk)
diff --git a/arch/arm/mach-imx/clock-imx35.c b/arch/arm/mach-imx/clock-imx35.c
index 5a4cc1e..2f80f14 100644
--- a/arch/arm/mach-imx/clock-imx35.c
+++ b/arch/arm/mach-imx/clock-imx35.c
@@ -458,9 +458,9 @@ static struct clk_lookup lookups[] = {
_REGISTER_CLOCK(imx-epit.0, NULL, epit1_clk)
_REGISTER_CLOCK(imx-epit.1, NULL, epit2_clk)
_REGISTER_CLOCK(NULL, esai, esai_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.0, NULL, esdhc1_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.1, NULL, esdhc2_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.2, NULL, esdhc3_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx35.0, NULL, esdhc1_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx35.1, NULL, esdhc2_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx35.2, NULL, esdhc3_clk)
_REGISTER_CLOCK(fec.0, NULL, fec_clk)
_REGISTER_CLOCK(NULL, gpio, gpio1_clk)
_REGISTER_CLOCK(NULL, gpio, gpio2_clk)
diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c
b/arch/arm/mach-mx5/clock-mx51-mx53.c
index 699b0d2..fd60e2c 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -1453,10 +1453,10 @@ static struct clk_lookup mx51_lookups[] = {
_REGISTER_CLOCK(imx51-ecspi.0, NULL, ecspi1_clk)
_REGISTER_CLOCK(imx51-ecspi.1, NULL, ecspi2_clk)
_REGISTER_CLOCK(imx51-cspi.0, NULL, cspi_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.0, NULL, esdhc1_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.1, NULL, esdhc2_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.2, NULL, esdhc3_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.3, NULL, esdhc4_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx51.0, NULL, esdhc1_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx51.1, NULL, esdhc2_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx51.2, NULL, esdhc3_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx51.3, NULL, esdhc4_clk)
_REGISTER_CLOCK(NULL, cpu_clk, cpu_clk)
_REGISTER_CLOCK(NULL, iim_clk, iim_clk)
_REGISTER_CLOCK(imx2-wdt.0, NULL, dummy_clk)
@@ -1480,10 +1480,10 @@ static struct clk_lookup mx53_lookups[] = {
_REGISTER_CLOCK(imx-i2c.0, NULL, i2c1_clk)
_REGISTER_CLOCK(imx-i2c.1, NULL, i2c2_clk)
_REGISTER_CLOCK(imx-i2c.2, NULL, i2c3_mx53_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.0, NULL, esdhc1_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.1, NULL, esdhc2_mx53_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.2, NULL, esdhc3_mx53_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.3, NULL, esdhc4_mx53_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx53.0, NULL, esdhc1_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx53.1, NULL, esdhc2_mx53_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx53.2, NULL, esdhc3_mx53_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx53.3, NULL, esdhc4_mx53_clk)
_REGISTER_CLOCK(imx53-ecspi.0, NULL, ecspi1_clk)
_REGISTER_CLOCK(imx53-ecspi.1, NULL, ecspi2_clk)
_REGISTER_CLOCK(imx53-cspi.0, NULL, cspi_clk)
diff --git a/arch/arm/mach-mx5/mx51_efika.c b/arch/arm/mach-mx5/mx51_efika.c
index 56739c2..4435e03 100644
--- a/arch/arm/mach-mx5/mx51_efika.c
+++ b/arch/arm/mach-mx5/mx51_efika.c
@@ -260,8 +260,8 @@ static struct regulator_consumer_supply
vvideo_consumers[] = {
};
static struct regulator_consumer_supply vsd_consumers
Re: [PATCH v2 3/3] mmc: sdhci-esdhc-imx: add device tree probe support
On Tue, Jul 5, 2011 at 9:26 AM, Shawn Guo shawn@linaro.org wrote:
The patch adds device tree probe support for sdhci-esdhc-imx driver.
Signed-off-by: Shawn Guo shawn@linaro.org
Cc: Wolfram Sang w.s...@pengutronix.de
Cc: Chris Ball c...@laptop.org
Cc: Grant Likely grant.lik...@secretlab.ca
---
.../devicetree/bindings/mmc/fsl-imx-esdhc.txt | 40
drivers/mmc/host/sdhci-esdhc-imx.c | 102
+++-
2 files changed, 137 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
new file mode 100644
index 000..351d239
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
@@ -0,0 +1,40 @@
+* Freescale Enhanced Secure Digital Host Controller (eSDHC) for i.MX
+
+The Enhanced Secure Digital Host Controller on Freescale i.MX family
+provides an interface for MMC, SD, and SDIO types of memory cards.
+
+Required properties:
+- compatible : Should be fsl,chip-esdhc
+- reg : Should contain eSDHC registers location and length
+- interrupts : Should contain eSDHC interrupt
+- cd-type : String, card detection (CD) method. Supported values are:
+ none : No CD
+ controller : Uses eSDHC controller internal CD signal
+ gpio : Uses GPIO pin for CD
+ permanent : No CD because card is permanently wired to host
+- wp-type : String, write protection (WP) method. Supported values are:
+ none : No WP
+ controller : Uses eSDHC controller internal WP signal
+ gpio : Uses GPIO pin for WP
+- gpios : Should specify GPIOs in this order: CD GPIO, WP GPIO, if
+ properties cd-type and wp-type are gpio.
Again, be explicit in your gpios property names. Create a different
property for each gpio: cd-gpios and wp-gpios.
As for wp-type and cd-type, I think you can drop them. Default to
internal controller CD and WP pins. Use gpio if cd-gpios or wp-gpios
is present, and define specific properties for the no-wp, no-cd and
fixed-card cases. (can you tell that I'm not a fan of the *-type
binding for this driver?) :-)
+
+Examples:
+
+esdhc@70004000 {
+ compatible = fsl,imx51-esdhc;
+ reg = 0x70004000 0x4000;
+ interrupts = 1;
+ fsl,cd-type = controller;
+ fsl,wp-type = controller;
+};
+
+esdhc@70008000 {
+ compatible = fsl,imx51-esdhc;
+ reg = 0x70008000 0x4000;
+ interrupts = 2;
+ fsl,cd-type = gpio;
+ fsl,wp-type = gpio;
+ cd-gpios = gpio0 6 0; /* GPIO1_6 */
+ wp-gpios = gpio0 5 0; /* GPIO1_5 */
+};
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index 1edda29..593d6b9 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -20,6 +20,9 @@
#include linux/mmc/host.h
#include linux/mmc/mmc.h
#include linux/mmc/sdio.h
+#include linux/of.h
+#include linux/of_device.h
+#include linux/of_gpio.h
#include mach/esdhc.h
#include sdhci-pltfm.h
#include sdhci-esdhc.h
@@ -72,6 +75,14 @@ static struct platform_device_id imx_esdhc_devtype[] = {
}
};
+static const struct of_device_id imx_esdhc_dt_ids[] = {
+ { .compatible = fsl,imx25-esdhc, .data =
imx_esdhc_devtype[IMX25_ESDHC], },
+ { .compatible = fsl,imx35-esdhc, .data =
imx_esdhc_devtype[IMX35_ESDHC], },
+ { .compatible = fsl,imx51-esdhc, .data =
imx_esdhc_devtype[IMX51_ESDHC], },
+ { .compatible = fsl,imx53-esdhc, .data =
imx_esdhc_devtype[IMX53_ESDHC], },
+ { /* sentinel */ }
+};
+
static inline int is_imx25_esdhc(struct pltfm_imx_data *data)
{
return data-devtype == IMX25_ESDHC;
@@ -305,24 +316,96 @@ static irqreturn_t cd_irq(int irq, void *data)
return IRQ_HANDLED;
};
+#ifdef CONFIG_OF
+static const char *cd_types[] = {
+ [ESDHC_CD_NONE] = none,
+ [ESDHC_CD_CONTROLLER] = controller,
+ [ESDHC_CD_GPIO] = gpio,
+ [ESDHC_CD_PERMANENT] = permanent,
+};
+
+static const char *wp_types[] = {
+ [ESDHC_WP_NONE] = none,
+ [ESDHC_WP_CONTROLLER] = controller,
+ [ESDHC_WP_GPIO] = gpio,
+};
+
+static int __devinit sdhci_esdhc_imx_probe_dt(struct platform_device *pdev)
+{
+ const struct of_device_id *of_id =
+ of_match_device(imx_esdhc_dt_ids, pdev-dev);
+ struct device_node *np = pdev-dev.of_node;
+ struct esdhc_platform_data *boarddata;
+ int err, i;
+ const char *cd, *wp;
+
+ if (!np)
+ return -ENODEV;
+
+ boarddata = kzalloc(sizeof(*boarddata), GFP_KERNEL);
+ if (!boarddata)
+ return -ENOMEM;
+ pdev-dev.platform_data = boarddata;
This is illegal. As far as device drivers are concerned,
pdev-dev.platform_data is immutable
Re: [PATCH 1/3] mmc: sdhci-esdhc-imx: get rid of the uses of cpu_is_mx()
On Sun, Jul 03, 2011 at 04:30:49PM +0800, Shawn Guo wrote:
The patch removes all the uses of cpu_is_mx(). Instead, it utilizes
platform_device_id to distinguish the esdhc differences among SoCs.
Signed-off-by: Shawn Guo shawn@linaro.org
Cc: Wolfram Sang w.s...@pengutronix.de
Cc: Chris Ball c...@laptop.org
Same comments apply to this patch as on the others. Minor
implementation detail comments that I won't bother repeating, but in
general I like the approach.
g.
---
arch/arm/mach-imx/clock-imx25.c|4 +-
arch/arm/mach-imx/clock-imx35.c|6 +-
arch/arm/mach-mx5/clock-mx51-mx53.c| 16
arch/arm/mach-mx5/mx51_efika.c |4 +-
.../plat-mxc/devices/platform-sdhci-esdhc-imx.c| 17
arch/arm/plat-mxc/include/mach/devices-common.h|1 +
drivers/mmc/host/sdhci-esdhc-imx.c | 45
++--
7 files changed, 66 insertions(+), 27 deletions(-)
diff --git a/arch/arm/mach-imx/clock-imx25.c b/arch/arm/mach-imx/clock-imx25.c
index a65838f..2955afa 100644
--- a/arch/arm/mach-imx/clock-imx25.c
+++ b/arch/arm/mach-imx/clock-imx25.c
@@ -300,8 +300,8 @@ static struct clk_lookup lookups[] = {
_REGISTER_CLOCK(imx2-wdt.0, NULL, wdt_clk)
_REGISTER_CLOCK(imx-ssi.0, NULL, ssi1_clk)
_REGISTER_CLOCK(imx-ssi.1, NULL, ssi2_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.0, NULL, esdhc1_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.1, NULL, esdhc2_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx25.0, NULL, esdhc1_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx25.1, NULL, esdhc2_clk)
_REGISTER_CLOCK(mx2-camera.0, NULL, csi_clk)
_REGISTER_CLOCK(NULL, audmux, audmux_clk)
_REGISTER_CLOCK(flexcan.0, NULL, can1_clk)
diff --git a/arch/arm/mach-imx/clock-imx35.c b/arch/arm/mach-imx/clock-imx35.c
index 5a4cc1e..2f80f14 100644
--- a/arch/arm/mach-imx/clock-imx35.c
+++ b/arch/arm/mach-imx/clock-imx35.c
@@ -458,9 +458,9 @@ static struct clk_lookup lookups[] = {
_REGISTER_CLOCK(imx-epit.0, NULL, epit1_clk)
_REGISTER_CLOCK(imx-epit.1, NULL, epit2_clk)
_REGISTER_CLOCK(NULL, esai, esai_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.0, NULL, esdhc1_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.1, NULL, esdhc2_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.2, NULL, esdhc3_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx35.0, NULL, esdhc1_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx35.1, NULL, esdhc2_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx35.2, NULL, esdhc3_clk)
_REGISTER_CLOCK(fec.0, NULL, fec_clk)
_REGISTER_CLOCK(NULL, gpio, gpio1_clk)
_REGISTER_CLOCK(NULL, gpio, gpio2_clk)
diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c
b/arch/arm/mach-mx5/clock-mx51-mx53.c
index 699b0d2..fd60e2c 100644
--- a/arch/arm/mach-mx5/clock-mx51-mx53.c
+++ b/arch/arm/mach-mx5/clock-mx51-mx53.c
@@ -1453,10 +1453,10 @@ static struct clk_lookup mx51_lookups[] = {
_REGISTER_CLOCK(imx51-ecspi.0, NULL, ecspi1_clk)
_REGISTER_CLOCK(imx51-ecspi.1, NULL, ecspi2_clk)
_REGISTER_CLOCK(imx51-cspi.0, NULL, cspi_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.0, NULL, esdhc1_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.1, NULL, esdhc2_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.2, NULL, esdhc3_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.3, NULL, esdhc4_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx51.0, NULL, esdhc1_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx51.1, NULL, esdhc2_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx51.2, NULL, esdhc3_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx51.3, NULL, esdhc4_clk)
_REGISTER_CLOCK(NULL, cpu_clk, cpu_clk)
_REGISTER_CLOCK(NULL, iim_clk, iim_clk)
_REGISTER_CLOCK(imx2-wdt.0, NULL, dummy_clk)
@@ -1480,10 +1480,10 @@ static struct clk_lookup mx53_lookups[] = {
_REGISTER_CLOCK(imx-i2c.0, NULL, i2c1_clk)
_REGISTER_CLOCK(imx-i2c.1, NULL, i2c2_clk)
_REGISTER_CLOCK(imx-i2c.2, NULL, i2c3_mx53_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.0, NULL, esdhc1_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.1, NULL, esdhc2_mx53_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.2, NULL, esdhc3_mx53_clk)
- _REGISTER_CLOCK(sdhci-esdhc-imx.3, NULL, esdhc4_mx53_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx53.0, NULL, esdhc1_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx53.1, NULL, esdhc2_mx53_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx53.2, NULL, esdhc3_mx53_clk)
+ _REGISTER_CLOCK(sdhci-esdhc-imx53.3, NULL, esdhc4_mx53_clk)
_REGISTER_CLOCK(imx53-ecspi.0, NULL, ecspi1_clk)
_REGISTER_CLOCK(imx53-ecspi.1, NULL, ecspi2_clk)
_REGISTER_CLOCK(imx53-cspi.0, NULL, cspi_clk)
diff --git a/arch/arm/mach-mx5/mx51_efika.c b/arch/arm/mach-mx5/mx51_efika.c
index 56739c2..4435e03 100644
--- a/arch/arm/mach-mx5/mx51_efika.c
+++ b/arch/arm/mach-mx5/mx51_efika.c
@@ -260,8 +260,8 @@ static struct regulator_consumer_supply
vvideo_consumers[] = {
};
static struct
Re: [PATCH 2/3] mmc: sdhci-pltfm: dt device does not pass parent to sdhci_alloc_host
On Sun, Jul 03, 2011 at 04:30:50PM +0800, Shawn Guo wrote:
Neither platform based nor dt based device needs to pass the parent
to sdhci_alloc_host. There is no difference between platform and dt
on this point.
The patch makes the change to pass device itself than its parent to
sdhci_alloc_host for dt case too. Otherwise the probe function of
sdhci based drivers which is shared between platform and dt will
fail on dt case.
Signed-off-by: Shawn Guo shawn@linaro.org
Cc: Chris Ball c...@laptop.org
Cc: Grant Likely grant.lik...@secretlab.ca
Acked-by: Grant Likely grant.lik...@secretlab.ca
---
drivers/mmc/host/sdhci-pltfm.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 71c0ce1..6414efe 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -85,6 +85,7 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device
*pdev,
{
struct sdhci_host *host;
struct sdhci_pltfm_host *pltfm_host;
+ struct device_node *np = pdev-dev.of_node;
struct resource *iomem;
int ret;
@@ -98,7 +99,7 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device
*pdev,
dev_err(pdev-dev, Invalid iomem size!\n);
/* Some PCI-based MFD need the parent here */
- if (pdev-dev.parent != platform_bus)
+ if (pdev-dev.parent != platform_bus !np)
host = sdhci_alloc_host(pdev-dev.parent, sizeof(*pltfm_host));
else
host = sdhci_alloc_host(pdev-dev, sizeof(*pltfm_host));
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] mmc: sdhci-esdhc-imx: add device tree probe support
On Sun, Jul 03, 2011 at 04:30:51PM +0800, Shawn Guo wrote:
The patch adds device tree probe support for sdhci-esdhc-imx driver.
Signed-off-by: Shawn Guo shawn@linaro.org
Cc: Wolfram Sang w.s...@pengutronix.de
Cc: Chris Ball c...@laptop.org
Cc: Grant Likely grant.lik...@secretlab.ca
---
.../devicetree/bindings/mmc/fsl-imx-esdhc.txt | 40
drivers/mmc/host/sdhci-esdhc-imx.c | 99
+++-
2 files changed, 134 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
new file mode 100644
index 000..e182e7c
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
@@ -0,0 +1,40 @@
+* Freescale Enhanced Secure Digital Host Controller (eSDHC) for i.MX
+
+The Enhanced Secure Digital Host Controller on Freescale i.MX family
+provides an interface for MMC, SD, and SDIO types of memory cards.
+
+Required properties:
+- compatible : Should be fsl,chip-esdhc
+- reg : Should contain eSDHC registers location and length
+- interrupts : Should contain eSDHC interrupt
+- cd-type : String, card detection (CD) method. Supported values are:
Similar to previous comments, use the fsl, prefix to this property name.
+none : No CD
+controller : Uses eSDHC controller internal CD signal
+gpio : Uses GPIO pin for CD
I would say the presence of a cd-gpios property would implicitly
mean gpio is to be used for the CD pin.
+permanent : No CD because card is permanently wired to host
+- wp-type : String, write protection (WP) method. Supported values are:
+none : No WP
+controller : Uses eSDHC controller internal WP signal
+gpio : Uses GPIO pin for WP
Ditto comments here.
+- gpios : Should specify GPIOs in this order: CD GPIO, WP GPIO, if
+ properties cd-type and wp-type are gpio.
The gpios binding has been extended to allow named gpios now. You can
uses something like cd-gpios and wp-gpios.
+
+Examples:
+
+esdhc@70004000 {
+ compatible = fsl,imx51-esdhc;
+ reg = 0x70004000 0x4000;
+ interrupts = 1;
+ cd-type = controller;
+ wp-type = controller;
+};
+
+esdhc@70008000 {
+ compatible = fsl,imx51-esdhc;
+ reg = 0x70008000 0x4000;
+ interrupts = 2;
+ cd-type = gpio;
+ wp-type = gpio;
+ gpios = gpio0 6 0, /* CD, GPIO1_6 */
+ gpio0 5 0; /* WP, GPIO1_5 */
+};
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index 705d670..57793e3 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -20,6 +20,9 @@
#include linux/mmc/host.h
#include linux/mmc/mmc.h
#include linux/mmc/sdio.h
+#include linux/of.h
+#include linux/of_device.h
+#include linux/of_gpio.h
#include mach/esdhc.h
#include sdhci-pltfm.h
#include sdhci-esdhc.h
@@ -78,6 +81,14 @@ static struct platform_device_id imx_esdhc_devtype[] = {
};
MODULE_DEVICE_TABLE(platform, imx_esdhc_devtype);
+static const struct of_device_id imx_esdhc_dt_ids[] = {
+ { .compatible = fsl,imx25-esdhc, .data =
imx_esdhc_devtype[IMX25_ESDHC], },
+ { .compatible = fsl,imx35-esdhc, .data =
imx_esdhc_devtype[IMX35_ESDHC], },
+ { .compatible = fsl,imx51-esdhc, .data =
imx_esdhc_devtype[IMX51_ESDHC], },
+ { .compatible = fsl,imx53-esdhc, .data =
imx_esdhc_devtype[IMX53_ESDHC], },
+ { /* sentinel */ },
+};
+
static unsigned int esdhc_pltfm_get_max_blk_size(struct sdhci_host *host)
{
/* Force 2048 bytes, which is the maximum supported size in SDHCI. */
@@ -290,24 +301,93 @@ static irqreturn_t cd_irq(int irq, void *data)
return IRQ_HANDLED;
};
+#ifdef CONFIG_OF
+static int __devinit sdhci_esdhc_imx_probe_dt(struct platform_device *pdev)
+{
+ const struct of_device_id *of_id =
+ of_match_device(imx_esdhc_dt_ids, pdev-dev);
+ struct device_node *np = pdev-dev.of_node;
+ struct esdhc_platform_data *boarddata;
+ int err, i;
+ const char *cd, *cd_types[] = {
+ [ESDHC_CD_NONE] = none,
+ [ESDHC_CD_CONTROLLER] = controller,
+ [ESDHC_CD_GPIO] = gpio,
+ [ESDHC_CD_PERMANENT]= permanent,
+ };
+ const char *wp, *wp_types[] = {
+ [ESDHC_WP_NONE] = none,
+ [ESDHC_WP_CONTROLLER] = controller,
+ [ESDHC_WP_GPIO] = gpio,
+ };
+
+ if (!np)
+ return -ENODEV;
+
+ boarddata = kzalloc(sizeof(*boarddata), GFP_KERNEL);
+ if (!boarddata)
+ return -ENOMEM;
+ pdev-dev.platform_data = boarddata;
+
+ err = of_property_read_string(np, cd-type, cd);
+ if (err)
+ return err;
+ for (i = 0; i ARRAY_SIZE(cd_types); i
Re: [PATCH] drivers:mmc:add the NO_IRQ define to of_mmc_spi.c
On Tue, May 24, 2011 at 09:03:22PM +0800, Wanlong Gao wrote:
For archs that don't support NO_IRQ (such as mips),
provide a dummy value. Fix the build error for mips.
Signed-off-by: Wanlong Gao wanlong@gmail.com
Acked-by: Grant Likely grant.lik...@secretlab.ca
---
drivers/mmc/host/of_mmc_spi.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/of_mmc_spi.c b/drivers/mmc/host/of_mmc_spi.c
index e2aecb7..ab66f24 100644
--- a/drivers/mmc/host/of_mmc_spi.c
+++ b/drivers/mmc/host/of_mmc_spi.c
@@ -25,6 +25,11 @@
#include linux/mmc/core.h
#include linux/mmc/host.h
+/* For archs that don't support NO_IRQ (such as mips), provide a dummy value
*/
+#ifndef NO_IRQ
+#define NO_IRQ 0
+#endif
+
MODULE_LICENSE(GPL);
enum {
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-mmc 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] Add OF binding helpers for MMC drivers
On Thu, Apr 21, 2011 at 03:19:57PM +0200, Domenico Andreoli wrote:
From: Domenico Andreoli cav...@gmail.com
This patch adds helpers to manage OF binding of MMC DeviceTree configs.
They don't cover all the MMC configuration cases, indeed are only a
slight generalization of those found in the MMC-over-SPI driver. More
will come later.
Can the mmc-over-spi driver be generalized to use this code too?
Signed-off-by: Domenico Andreoli cav...@gmail.com
---
drivers/of/Kconfig |4 +
drivers/of/Makefile|1 +
drivers/of/of_mmc.c| 165
+
include/linux/of_mmc.h | 50 +++
Personally I think it makes more sense for this code to live in
drivers/mmc/core.
4 files changed, 220 insertions(+)
Index: b/drivers/of/Kconfig
===
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -59,6 +59,10 @@ config OF_I2C
help
OpenFirmware I2C accessors
+config OF_MMC
+ depends on MMC
+ def_bool y
+
config OF_NET
depends on NETDEVICES
def_bool y
Index: b/drivers/of/Makefile
===
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_OF_DEVICE) += device.o plat
obj-$(CONFIG_OF_GPIO) += gpio.o
obj-$(CONFIG_OF_CLOCK) += clock.o
obj-$(CONFIG_OF_I2C) += of_i2c.o
+obj-$(CONFIG_OF_MMC) += of_mmc.o
obj-$(CONFIG_OF_NET) += of_net.o
obj-$(CONFIG_OF_SPI) += of_spi.o
obj-$(CONFIG_OF_MDIO)+= of_mdio.o
Index: b/drivers/of/of_mmc.c
===
--- /dev/null
+++ b/drivers/of/of_mmc.c
@@ -0,0 +1,165 @@
+/*
+ * OF helpers for the MMC API
+ *
+ * Copyright (c) 2011 Domenico Andreoli
+ *
+ * Heavily inspired by the OF support to the MMC-over-SPI driver made
+ * by Anton Vorontsov
+ *
+ * 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.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/device.h
+#include linux/err.h
+#include linux/slab.h
+#include linux/mmc/core.h
+#include linux/gpio.h
+#include linux/of.h
+#include linux/of_mmc.h
+#include linux/of_gpio.h
+
+static int of_read_mmc_gpio(struct of_mmc_crg *crg, int gpio_num)
+{
+ int value, active_low;
+
+ BUG_ON(gpio_num = NUM_MMC_GPIOS);
+
+ /* hitting this means that DeviceTree left this gpio unspecified
+ * by purpose but driver didn't take any measure to define its
+ * behavior (i.e. aborting probe phase or disabling the feature).
+ * driver needs to call of_is_valid_mmc_crg() for each expected
+ * gpio to detect this case.
+ */
+ if (WARN_ON(crg-gpios[gpio_num] 0))
+ return -1;
+
+ value = gpio_get_value(crg-gpios[gpio_num]);
+ active_low = crg-alow_gpios[gpio_num];
+ return value ^ active_low;
+}
+
+int of_get_mmc_cd_gpio(struct of_mmc_crg *crg)
+{
+ return of_read_mmc_gpio(crg, CD_MMC_GPIO);
+}
+EXPORT_SYMBOL(of_get_mmc_cd_gpio);
+
+int of_get_mmc_ro_gpio(struct of_mmc_crg *crg)
+{
+ return of_read_mmc_gpio(crg, WP_MMC_GPIO);
+}
+EXPORT_SYMBOL(of_get_mmc_ro_gpio);
+
+int of_is_valid_mmc_crg(struct of_mmc_crg *crg, int gpio_num)
+{
+ BUG_ON(gpio_num = NUM_MMC_GPIOS);
+ return gpio_is_valid(crg-gpios[gpio_num]);
+}
+EXPORT_SYMBOL(of_is_valid_mmc_crg);
+
+int of_get_mmc_crg(struct device *dev, struct device_node *np,
+ int cd_off, struct of_mmc_crg *crg)
+{
+ int *gpio, *alow;
+ int i, ret;
+
+ memset(crg, 0, sizeof(*crg));
+ crg-cd_irq = -1;
+
+ gpio = crg-gpios;
+ alow = crg-alow_gpios;
+ for (i = 0; i NUM_MMC_GPIOS; i++, gpio++, alow++) {
+ enum of_gpio_flags gpio_flags;
+ *gpio = of_get_gpio_flags(np, cd_off+i, gpio_flags);
+ *alow = !!(gpio_flags OF_GPIO_ACTIVE_LOW);
+
+ if (*gpio == -EEXIST || *gpio == -ENOENT) {
+ /* driver needs to define proper meaning of this missing
+gpio (i.e. abort probe or disable the feature) */
+ pr_debug(%s: gpio #%d is not specified\n, __func__,
i);
+ continue;
+ }
+ if (*gpio 0) {
+ pr_debug(%s: invalid configuration\n, __func__);
+ ret = *gpio;
+ break;
+ }
+ if (!gpio_is_valid(*gpio)) {
+ pr_debug(%s: gpio #%d is not valid: %d\n, __func__,
i, *gpio);
+ ret = -EINVAL;
+ break;
+ }
+ ret =
Re: [PATCH 0/4] mmc_spi: Add support for regulator framework
On Tue, Apr 05, 2011 at 10:43:47AM +0200, Antonio Ospite wrote: On Mon, 4 Apr 2011 21:05:43 -0600 Grant Likely grant.lik...@secretlab.ca wrote: On Mon, Apr 04, 2011 at 11:56:31AM +0200, Antonio Ospite wrote: On Mon, 21 Mar 2011 19:46:38 +0100 Antonio Ospite osp...@studenti.unina.it wrote: Hi, this patchset has the purpose of adding support for the regulator framework to the mmc_spi driver. The first three patches are preparatory cleanups to make the fourth one more straightforward. Maybe the fourth patch can be improved, I am open to any suggestions about it. Ping. I forgot to Cc spi-devel-general on this series, should I resend it? Not a bad idea. It doesn't go via my tree since it is an mmc patch, not an spi one, but I don't mind taking a look at the spi bits. Grant you were on Cc from the start so you should have the patches somewhere; please tell me if you don't. I'd avoid resending if not strictly necessary. Ah, then I probably scanned it briefly and decided I didn't need to respond to it. Don't worry about reposting. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] mmc_spi: Add support for regulator framework
On Mon, Apr 04, 2011 at 11:56:31AM +0200, Antonio Ospite wrote: On Mon, 21 Mar 2011 19:46:38 +0100 Antonio Ospite osp...@studenti.unina.it wrote: Hi, this patchset has the purpose of adding support for the regulator framework to the mmc_spi driver. The first three patches are preparatory cleanups to make the fourth one more straightforward. Maybe the fourth patch can be improved, I am open to any suggestions about it. Ping. I forgot to Cc spi-devel-general on this series, should I resend it? Not a bad idea. It doesn't go via my tree since it is an mmc patch, not an spi one, but I don't mind taking a look at the spi bits. g. These changes take strong inspiration from the pxamci driver; they have been tested on a Motorola A910, which uses a regulator to powerup the MMC card connected to the SPI bus, a test from a current user of the mmc_spi driver would not hurt just to be sure no regressions have been introduced. Thanks, Antonio Antonio Ospite (4): mmc_spi.c: factor out the check for power capability mmc_spi.c: factor out the SD card shutdown sequence mmc_spi.c: factor out a mmc_spi_setpower() function mmc_spi.c: add support for the regulator framework drivers/mmc/host/mmc_spi.c | 194 +--- 1 files changed, 129 insertions(+), 65 deletions(-) -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? -- To unsubscribe from this list: send the line unsubscribe linux-mmc 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/5] mmc: sdhci: make sdhci-pltfm device drivers self registered
On Fri, Mar 25, 2011 at 04:48:47PM +0800, Shawn Guo wrote:
The patch turns the common stuff in sdhci-pltfm.c into functions, and
add device drivers their own .probe and .remove which in turn call
into the common functions, so that those sdhci-pltfm device drivers
register itself and keep all device specific things away from common
sdhci-pltfm file.
Signed-off-by: Shawn Guo shawn@linaro.org
Looks really good. Relatively minor comments below, but you can add
this to the next version:
Reviewed-by: Grant Likely grant.lik...@secretlab.ca
Thanks for doing this!
g.
---
drivers/mmc/host/Kconfig | 24 +++--
drivers/mmc/host/Makefile | 11 +-
drivers/mmc/host/sdhci-cns3xxx.c | 67 +-
drivers/mmc/host/sdhci-dove.c | 69 +-
drivers/mmc/host/sdhci-esdhc-imx.c | 97 +++
drivers/mmc/host/sdhci-pltfm.c | 165 ++-
drivers/mmc/host/sdhci-pltfm.h | 14 ++-
drivers/mmc/host/sdhci-tegra.c | 187
+---
include/linux/mmc/sdhci-pltfm.h|6 -
9 files changed, 360 insertions(+), 280 deletions(-)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index afe8c6f..1db9347 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -113,20 +113,17 @@ config MMC_SDHCI_OF_HLWD
If unsure, say N.
config MMC_SDHCI_PLTFM
- tristate SDHCI support on the platform specific bus
+ bool
depends on MMC_SDHCI
help
- This selects the platform specific bus support for Secure Digital Host
- Controller Interface.
-
- If you have a controller with this interface, say Y or M here.
-
- If unsure, say N.
+ This selects the platform common function support for Secure Digital
+ Host Controller Interface.
config MMC_SDHCI_CNS3XXX
bool SDHCI support on the Cavium Networks CNS3xxx SoC
depends on ARCH_CNS3XXX
- depends on MMC_SDHCI_PLTFM
+ depends on MMC_SDHCI
+ select MMC_SDHCI_PLTFM
help
This selects the SDHCI support for CNS3xxx System-on-Chip devices.
@@ -134,7 +131,9 @@ config MMC_SDHCI_CNS3XXX
config MMC_SDHCI_ESDHC_IMX
bool SDHCI platform support for the Freescale eSDHC i.MX controller
- depends on MMC_SDHCI_PLTFM (ARCH_MX25 || ARCH_MX35 || ARCH_MX5)
+ depends on ARCH_MX25 || ARCH_MX35 || ARCH_MX5
+ depends on MMC_SDHCI
+ select MMC_SDHCI_PLTFM
select MMC_SDHCI_IO_ACCESSORS
help
This selects the Freescale eSDHC controller support on the platform
@@ -145,7 +144,8 @@ config MMC_SDHCI_ESDHC_IMX
config MMC_SDHCI_DOVE
bool SDHCI support on Marvell's Dove SoC
depends on ARCH_DOVE
- depends on MMC_SDHCI_PLTFM
+ depends on MMC_SDHCI
+ select MMC_SDHCI_PLTFM
select MMC_SDHCI_IO_ACCESSORS
help
This selects the Secure Digital Host Controller Interface in
@@ -155,7 +155,9 @@ config MMC_SDHCI_DOVE
config MMC_SDHCI_TEGRA
tristate SDHCI platform support for the Tegra SD/MMC Controller
- depends on MMC_SDHCI_PLTFM ARCH_TEGRA
+ depends on ARCH_TEGRA
+ depends on MMC_SDHCI
+ select MMC_SDHCI_PLTFM
select MMC_SDHCI_IO_ACCESSORS
help
This selects the Tegra SD/MMC controller. If you have a Tegra
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index e834fb2..1d8e43d 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -36,12 +36,11 @@ obj-$(CONFIG_MMC_SH_MMCIF)+= sh_mmcif.o
obj-$(CONFIG_MMC_JZ4740) += jz4740_mmc.o
obj-$(CONFIG_MMC_USHC) += ushc.o
-obj-$(CONFIG_MMC_SDHCI_PLTFM)+= sdhci-platform.o
-sdhci-platform-y := sdhci-pltfm.o
-sdhci-platform-$(CONFIG_MMC_SDHCI_CNS3XXX) += sdhci-cns3xxx.o
-sdhci-platform-$(CONFIG_MMC_SDHCI_ESDHC_IMX) += sdhci-esdhc-imx.o
-sdhci-platform-$(CONFIG_MMC_SDHCI_DOVE) += sdhci-dove.o
-sdhci-platform-$(CONFIG_MMC_SDHCI_TEGRA) += sdhci-tegra.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
+obj-$(CONFIG_MMC_SDHCI_DOVE) += sdhci-dove.o
+obj-$(CONFIG_MMC_SDHCI_TEGRA)+= sdhci-tegra.o
obj-$(CONFIG_MMC_SDHCI_OF) += sdhci-of.o
sdhci-of-y := sdhci-of-core.o
diff --git a/drivers/mmc/host/sdhci-cns3xxx.c
b/drivers/mmc/host/sdhci-cns3xxx.c
index 9ebd1d7..95b9080 100644
--- a/drivers/mmc/host/sdhci-cns3xxx.c
+++ b/drivers/mmc/host/sdhci-cns3xxx.c
@@ -86,7 +86,7 @@ static struct sdhci_ops sdhci_cns3xxx_ops = {
.set_clock = sdhci_cns3xxx_set_clock,
};
-struct sdhci_pltfm_data sdhci_cns3xxx_pdata = {
+static struct sdhci_pltfm_data sdhci_cns3xxx_pdata = {
.ops
Re: [PATCH 2/5] mmc: sdhci: eliminate sdhci_of_host and sdhci_of_data
On Fri, Mar 25, 2011 at 04:48:48PM +0800, Shawn Guo wrote:
The patch is to migrate the use of sdhci_of_host and sdhci_of_data
to sdhci_pltfm_host and sdhci_pltfm_data, so that the former pair can
be eliminated.
Signed-off-by: Shawn Guo shawn@linaro.org
Reviewed-by: Grant Likely grant.lik...@secretlab.ca
---
drivers/mmc/host/sdhci-of-core.c | 30 +++---
drivers/mmc/host/sdhci-of-esdhc.c | 36 +++-
drivers/mmc/host/sdhci-of-hlwd.c | 20 +++-
drivers/mmc/host/sdhci-of.h | 15 +++
drivers/mmc/host/sdhci-pltfm.h|4
5 files changed, 52 insertions(+), 53 deletions(-)
diff --git a/drivers/mmc/host/sdhci-of-core.c
b/drivers/mmc/host/sdhci-of-core.c
index dd84124..a6c0132 100644
--- a/drivers/mmc/host/sdhci-of-core.c
+++ b/drivers/mmc/host/sdhci-of-core.c
@@ -59,7 +59,7 @@ void sdhci_be32bs_writel(struct sdhci_host *host, u32 val,
int reg)
void sdhci_be32bs_writew(struct sdhci_host *host, u16 val, int reg)
{
- struct sdhci_of_host *of_host = sdhci_priv(host);
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
int base = reg ~0x3;
int shift = (reg 0x2) * 8;
@@ -69,10 +69,10 @@ void sdhci_be32bs_writew(struct sdhci_host *host, u16
val, int reg)
* Postpone this write, we must do it together with a
* command write that is down below.
*/
- of_host-xfer_mode_shadow = val;
+ pltfm_host-xfer_mode_shadow = val;
return;
case SDHCI_COMMAND:
- sdhci_be32bs_writel(host, val 16 | of_host-xfer_mode_shadow,
+ sdhci_be32bs_writel(host, val 16 |
pltfm_host-xfer_mode_shadow,
SDHCI_TRANSFER_MODE);
return;
}
@@ -128,9 +128,9 @@ static int __devinit sdhci_of_probe(struct
platform_device *ofdev,
const struct of_device_id *match)
{
struct device_node *np = ofdev-dev.of_node;
- struct sdhci_of_data *sdhci_of_data = match-data;
+ struct sdhci_pltfm_data *pdata = match-data;
struct sdhci_host *host;
- struct sdhci_of_host *of_host;
+ struct sdhci_pltfm_host *pltfm_host;
const __be32 *clk;
int size;
int ret;
@@ -138,11 +138,11 @@ static int __devinit sdhci_of_probe(struct
platform_device *ofdev,
if (!of_device_is_available(np))
return -ENODEV;
- host = sdhci_alloc_host(ofdev-dev, sizeof(*of_host));
+ host = sdhci_alloc_host(ofdev-dev, sizeof(*pltfm_host));
if (IS_ERR(host))
return -ENOMEM;
- of_host = sdhci_priv(host);
+ pltfm_host = sdhci_priv(host);
dev_set_drvdata(ofdev-dev, host);
host-ioaddr = of_iomap(np, 0);
@@ -158,9 +158,9 @@ static int __devinit sdhci_of_probe(struct
platform_device *ofdev,
}
host-hw_name = dev_name(ofdev-dev);
- if (sdhci_of_data) {
- host-quirks = sdhci_of_data-quirks;
- host-ops = sdhci_of_data-ops;
+ if (pdata) {
+ host-quirks = pdata-quirks;
+ host-ops = pdata-ops;
}
if (of_get_property(np, sdhci,auto-cmd12, NULL))
@@ -175,7 +175,7 @@ static int __devinit sdhci_of_probe(struct
platform_device *ofdev,
clk = of_get_property(np, clock-frequency, size);
if (clk size == sizeof(*clk) *clk)
- of_host-clock = be32_to_cpup(clk);
+ pltfm_host-clock = be32_to_cpup(clk);
ret = sdhci_add_host(host);
if (ret)
@@ -205,12 +205,12 @@ static int __devexit sdhci_of_remove(struct
platform_device *ofdev)
static const struct of_device_id sdhci_of_match[] = {
#ifdef CONFIG_MMC_SDHCI_OF_ESDHC
- { .compatible = fsl,mpc8379-esdhc, .data = sdhci_esdhc, },
- { .compatible = fsl,mpc8536-esdhc, .data = sdhci_esdhc, },
- { .compatible = fsl,esdhc, .data = sdhci_esdhc, },
+ { .compatible = fsl,mpc8379-esdhc, .data = sdhci_esdhc_pdata, },
+ { .compatible = fsl,mpc8536-esdhc, .data = sdhci_esdhc_pdata, },
+ { .compatible = fsl,esdhc, .data = sdhci_esdhc_pdata, },
#endif
#ifdef CONFIG_MMC_SDHCI_OF_HLWD
- { .compatible = nintendo,hollywood-sdhci, .data = sdhci_hlwd, },
+ { .compatible = nintendo,hollywood-sdhci, .data = sdhci_hlwd_pdata,
},
#endif
{ .compatible = generic-sdhci, },
{},
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
b/drivers/mmc/host/sdhci-of-esdhc.c
index fcd0e1f..702a98b 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -60,30 +60,32 @@ static int esdhc_of_enable_dma(struct sdhci_host *host)
static unsigned int esdhc_of_get_max_clock(struct sdhci_host *host)
{
- struct sdhci_of_host *of_host = sdhci_priv(host);
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
- return
Re: [PATCH 3/5] mmc: sdhci: make sdhci-of device drivers self registered
On Fri, Mar 25, 2011 at 04:48:49PM +0800, Shawn Guo wrote:
The patch turns the sdhci-of-core common stuff into helper functions
added into sdhci-pltfm.c, and makes sdhci-of device drviers self
registered using the same pair of .probe and .remove used by
sdhci-pltfm device drivers.
As a result, sdhci-of-core.c and sdhci-of.h can be eliminated with
those common things merged into sdhci-pltfm.c and sdhci-pltfm.h
respectively.
Signed-off-by: Shawn Guo shawn@linaro.org
---
drivers/mmc/host/Kconfig | 15 +--
drivers/mmc/host/Makefile |7 +-
drivers/mmc/host/sdhci-of-core.c | 247
-
drivers/mmc/host/sdhci-of-esdhc.c | 75 +++-
drivers/mmc/host/sdhci-of-hlwd.c | 73 +++-
drivers/mmc/host/sdhci-of.h | 33 -
drivers/mmc/host/sdhci-pltfm.c| 100 +++-
drivers/mmc/host/sdhci-pltfm.h| 14 ++
8 files changed, 263 insertions(+), 301 deletions(-)
delete mode 100644 drivers/mmc/host/sdhci-of-core.c
delete mode 100644 drivers/mmc/host/sdhci-of.h
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 1db9347..9f360b5 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -81,19 +81,11 @@ config MMC_RICOH_MMC
If unsure, say Y.
-config MMC_SDHCI_OF
- tristate SDHCI support on OpenFirmware platforms
- depends on MMC_SDHCI OF
- help
- This selects the OF support for Secure Digital Host Controller
- Interfaces.
-
- If unsure, say N.
-
config MMC_SDHCI_OF_ESDHC
bool SDHCI OF support for the Freescale eSDHC controller
- depends on MMC_SDHCI_OF
+ depends on MMC_SDHCI
depends on PPC_OF
+ select MMC_SDHCI_PLTFM
select MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
help
This selects the Freescale eSDHC controller support.
@@ -102,8 +94,9 @@ config MMC_SDHCI_OF_ESDHC
config MMC_SDHCI_OF_HLWD
bool SDHCI OF support for the Nintendo Wii SDHCI controllers
- depends on MMC_SDHCI_OF
+ depends on MMC_SDHCI
depends on PPC_OF
+ select MMC_SDHCI_PLTFM
select MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
help
This selects the Secure Digital Host Controller Interface (SDHCI)
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 1d8e43d..0ea8815 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -41,11 +41,8 @@ obj-$(CONFIG_MMC_SDHCI_CNS3XXX)+=
sdhci-cns3xxx.o
obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX)+= sdhci-esdhc-imx.o
obj-$(CONFIG_MMC_SDHCI_DOVE) += sdhci-dove.o
obj-$(CONFIG_MMC_SDHCI_TEGRA)+= sdhci-tegra.o
-
-obj-$(CONFIG_MMC_SDHCI_OF) += sdhci-of.o
-sdhci-of-y := sdhci-of-core.o
-sdhci-of-$(CONFIG_MMC_SDHCI_OF_ESDHC)+= sdhci-of-esdhc.o
-sdhci-of-$(CONFIG_MMC_SDHCI_OF_HLWD) += sdhci-of-hlwd.o
+obj-$(CONFIG_MMC_SDHCI_OF_ESDHC) += sdhci-of-esdhc.o
+obj-$(CONFIG_MMC_SDHCI_OF_HLWD) += sdhci-of-hlwd.o
ifeq ($(CONFIG_CB710_DEBUG),y)
CFLAGS-cb710-mmc+= -DDEBUG
diff --git a/drivers/mmc/host/sdhci-of-core.c
b/drivers/mmc/host/sdhci-of-core.c
deleted file mode 100644
index a6c0132..000
--- a/drivers/mmc/host/sdhci-of-core.c
+++ /dev/null
@@ -1,247 +0,0 @@
-/*
- * OpenFirmware bindings for Secure Digital Host Controller Interface.
- *
- * Copyright (c) 2007 Freescale Semiconductor, Inc.
- * Copyright (c) 2009 MontaVista Software, Inc.
- *
- * Authors: Xiaobo Xie x@freescale.com
- * Anton Vorontsov avoront...@ru.mvista.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.
- */
-
-#include linux/err.h
-#include linux/module.h
-#include linux/init.h
-#include linux/io.h
-#include linux/interrupt.h
-#include linux/delay.h
-#include linux/of.h
-#include linux/of_platform.h
-#include linux/of_address.h
-#include linux/of_irq.h
-#include linux/mmc/host.h
-#ifdef CONFIG_PPC
-#include asm/machdep.h
-#endif
-#include sdhci-of.h
-#include sdhci.h
-
-#ifdef CONFIG_MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
-
-/*
- * These accessors are designed for big endian hosts doing I/O to
- * little endian controllers incorporating a 32-bit hardware byte swapper.
- */
-
-u32 sdhci_be32bs_readl(struct sdhci_host *host, int reg)
-{
- return in_be32(host-ioaddr + reg);
-}
-
-u16 sdhci_be32bs_readw(struct sdhci_host *host, int reg)
-{
- return in_be16(host-ioaddr + (reg ^ 0x2));
-}
-
-u8 sdhci_be32bs_readb(struct sdhci_host *host, int reg)
-{
- return in_8(host-ioaddr + (reg ^ 0x3));
-}
-
-void sdhci_be32bs_writel(struct sdhci_host *host, u32 val, int reg)
-{
- out_be32(host-ioaddr +
Re: [PATCH 4/5] mmc: sdhci: consolidate sdhci-of-esdhc and sdhci-esdhc-imx
On Fri, Mar 25, 2011 at 04:48:50PM +0800, Shawn Guo wrote:
This patch is to consolidate SDHCI driver for Freescale eSDHC
controller found on both MPCxxx and i.MX platforms. It turns
sdhci-of-esdhc.c and sdhci-esdhc-imx.c into one sdhci-esdhc.c,
which gets the same pair of .probe and .remove serving two cases.
Signed-off-by: Shawn Guo shawn@linaro.org
---
drivers/mmc/host/Kconfig | 38 ++--
drivers/mmc/host/Makefile |3 +-
drivers/mmc/host/sdhci-esdhc-imx.c | 210 --
drivers/mmc/host/sdhci-esdhc.c | 413
drivers/mmc/host/sdhci-of-esdhc.c | 162 --
This patch would be easier to review if it was split into two patches;
first rename sdhci-esdhc-imx.c to sdhci-esdhc.c without any changes to
the .c code, and then a second patch to merge the ppc bits into the
imx version.
+#if defined(CONFIG_OF)
+#include linux/of_device.h
+static const struct of_device_id sdhci_esdhc_dt_ids[] = {
+#ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
+ { .compatible = fsl,imx-esdhc, .data = sdhci_esdhc_imx_pdata },
+#endif
+#ifdef CONFIG_MMC_SDHCI_ESDHC_MPC
+ { .compatible = fsl,mpc8379-esdhc, .data = sdhci_esdhc_mpc_pdata },
+ { .compatible = fsl,mpc8536-esdhc, .data = sdhci_esdhc_mpc_pdata },
+ { .compatible = fsl,esdhc, .data = sdhci_esdhc_mpc_pdata },
+#endif
+ { }
+};
+MODULE_DEVICE_TABLE(platform, sdhci_esdhc_dt_ids);
+
+static const struct of_device_id *
+sdhci_esdhc_get_of_device_id(struct platform_device *pdev)
+{
+ return of_match_device(sdhci_esdhc_dt_ids, pdev-dev);
You can add an empty implementation of of_match_device() to
linux/of_device.h. That would eliminate the need for this function.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] mmc: sdhci: merge two sdhci-pltfm.h into one
On Fri, Mar 25, 2011 at 04:48:51PM +0800, Shawn Guo wrote:
The structure sdhci_pltfm_data is not necessarily to be in a public
header like include/linux/mmc/sdhci-pltfm.h, so the patch moves it
into drivers/mmc/host/sdhci-pltfm.h and eliminates the former one.
Signed-off-by: Shawn Guo shawn@linaro.org
Reviewed-by: Grant Likely grant.lik...@secretlab.ca
Looks good to me.
---
drivers/mmc/host/sdhci-cns3xxx.c |1 -
drivers/mmc/host/sdhci-esdhc.c |1 -
drivers/mmc/host/sdhci-pltfm.h |6 +-
include/linux/mmc/sdhci-pltfm.h | 29 -
4 files changed, 5 insertions(+), 32 deletions(-)
delete mode 100644 include/linux/mmc/sdhci-pltfm.h
diff --git a/drivers/mmc/host/sdhci-cns3xxx.c
b/drivers/mmc/host/sdhci-cns3xxx.c
index 95b9080..2238d34 100644
--- a/drivers/mmc/host/sdhci-cns3xxx.c
+++ b/drivers/mmc/host/sdhci-cns3xxx.c
@@ -15,7 +15,6 @@
#include linux/delay.h
#include linux/device.h
#include linux/mmc/host.h
-#include linux/mmc/sdhci-pltfm.h
#include mach/cns3xxx.h
#include sdhci.h
#include sdhci-pltfm.h
diff --git a/drivers/mmc/host/sdhci-esdhc.c b/drivers/mmc/host/sdhci-esdhc.c
index b3d1bc1..fd041d9 100644
--- a/drivers/mmc/host/sdhci-esdhc.c
+++ b/drivers/mmc/host/sdhci-esdhc.c
@@ -20,7 +20,6 @@
#include linux/err.h
#include linux/clk.h
#include linux/mmc/host.h
-#include linux/mmc/sdhci-pltfm.h
#ifdef CONFIG_MMC_SDHCI_ESDHC_IMX
#include mach/hardware.h
#endif
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index 05fe25d..e2d143c 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -14,9 +14,13 @@
#include linux/clk.h
#include linux/types.h
#include linux/platform_device.h
-#include linux/mmc/sdhci-pltfm.h
#include linux/mmc/sdhci.h
+struct sdhci_pltfm_data {
+ struct sdhci_ops *ops;
+ unsigned int quirks;
+};
+
struct sdhci_pltfm_host {
struct clk *clk;
u32 scratchpad; /* to handle quirks across io-accessor calls */
diff --git a/include/linux/mmc/sdhci-pltfm.h b/include/linux/mmc/sdhci-pltfm.h
deleted file mode 100644
index f1c2ac3..000
--- a/include/linux/mmc/sdhci-pltfm.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/*
- * Platform data declarations for the sdhci-pltfm driver.
- *
- * Copyright (c) 2010 MontaVista Software, LLC.
- *
- * Author: Anton Vorontsov avoront...@ru.mvista.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.
- */
-
-#ifndef _SDHCI_PLTFM_H
-#define _SDHCI_PLTFM_H
-
-struct sdhci_ops;
-
-/**
- * struct sdhci_pltfm_data - SDHCI platform-specific information hooks
- * @ops: optional pointer to the platform-provided SDHCI ops
- * @quirks: optional SDHCI quirks
- */
-struct sdhci_pltfm_data {
- struct sdhci_ops *ops;
- unsigned int quirks;
-};
-
-#endif /* _SDHCI_PLTFM_H */
--
1.7.1
___
linaro-dev mailing list
linaro-...@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
--
To unsubscribe from this list: send the line unsubscribe linux-mmc 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/5] mmc: sdhci: make sdhci-esdhc-imx driver self registered
On Mon, Mar 21, 2011 at 04:06:59PM +0800, Shawn Guo wrote:
The patch turns the common stuff to in sdhci-pltfm.c into functions,
and add sdhci-esdhc-imx its own .probe and .remove which in turn call
into the common functions, so that sdhci-esdhc-imx driver registers
itself and keep all sdhci-esdhc-imx specific things like
sdhci_esdhc_imx_pdata away from sdhci-pltfm.c which is common.
Signed-off-by: Shawn Guo shawn@linaro.org
Hi Shawn,
Thanks for all this work, I think it is the right thing to do. A few
relatively minor comments below, but otherwise I like it.
g.
---
drivers/mmc/host/sdhci-esdhc-imx.c | 98
+---
drivers/mmc/host/sdhci-pltfm.c | 84 +--
drivers/mmc/host/sdhci-pltfm.h | 11 -
I think this patch should be split in 2. One patch to refactor
edhci-pltfm* to create the common utility functions, and one patch to
convert the imx driver.
3 files changed, 169 insertions(+), 24 deletions(-)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index 9b82910..6585620 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -100,15 +100,39 @@ static unsigned int esdhc_pltfm_get_min_clock(struct
sdhci_host *host)
return clk_get_rate(pltfm_host-clk) / 256 / 16;
}
-static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data
*pdata)
+static struct sdhci_ops sdhci_esdhc_ops = {
+ .read_w = esdhc_readw_le,
+ .write_w = esdhc_writew_le,
+ .write_b = esdhc_writeb_le,
+ .set_clock = esdhc_set_clock,
+ .get_max_clock = esdhc_pltfm_get_max_clock,
+ .get_min_clock = esdhc_pltfm_get_min_clock,
+};
+
+static struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
+ .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA,
+ /* ADMA has issues. Might be fixable */
+ .ops = sdhci_esdhc_ops,
+};
+
+static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
{
- struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_pltfm_host *pltfm_host;
+ struct sdhci_host *host;
struct clk *clk;
+ int ret;
+
+ host = sdhci_pltfm_init(pdev, sdhci_esdhc_imx_pdata);
Nice! I like that this adds type checking to the passing of the
sdhci_pltfm_data pointer.
+ if (!host)
+ return -ENOMEM;
+
+ pltfm_host = sdhci_priv(host);
clk = clk_get(mmc_dev(host-mmc), NULL);
if (IS_ERR(clk)) {
dev_err(mmc_dev(host-mmc), clk err\n);
- return PTR_ERR(clk);
+ ret = PTR_ERR(clk);
+ goto err_clk_get;
}
clk_enable(clk);
pltfm_host-clk = clk;
@@ -120,30 +144,68 @@ static int esdhc_pltfm_init(struct sdhci_host *host,
struct sdhci_pltfm_data *pd
if (cpu_is_mx25() || cpu_is_mx35())
host-quirks |= SDHCI_QUIRK_NO_MULTIBLOCK;
+ ret = sdhci_add_host(host);
+ if (ret)
+ goto err_add_host;
+
return 0;
+
+err_add_host:
+ clk_disable(pltfm_host-clk);
+ clk_put(pltfm_host-clk);
+err_clk_get:
+ sdhci_pltfm_free(pdev);
+ return ret;
}
-static void esdhc_pltfm_exit(struct sdhci_host *host)
+static int __devexit sdhci_esdhc_imx_remove(struct platform_device *pdev)
{
+ struct sdhci_host *host = platform_get_drvdata(pdev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ int dead = 0;
+ u32 scratch;
+
+ dead = 0;
+ scratch = readl(host-ioaddr + SDHCI_INT_STATUS);
+ if (scratch == (u32)-1)
+ dead = 1;
+
+ sdhci_remove_host(host, dead);
clk_disable(pltfm_host-clk);
clk_put(pltfm_host-clk);
+
+ sdhci_pltfm_free(pdev);
+
+ return 0;
}
-static struct sdhci_ops sdhci_esdhc_ops = {
- .read_w = esdhc_readw_le,
- .write_w = esdhc_writew_le,
- .write_b = esdhc_writeb_le,
- .set_clock = esdhc_set_clock,
- .get_max_clock = esdhc_pltfm_get_max_clock,
- .get_min_clock = esdhc_pltfm_get_min_clock,
+static struct platform_driver sdhci_esdhc_imx_driver = {
+ .driver = {
+ .name = sdhci-esdhc-imx,
+ .owner = THIS_MODULE,
+ },
+ .probe = sdhci_esdhc_imx_probe,
+ .remove = __devexit_p(sdhci_esdhc_imx_remove),
+#ifdef CONFIG_PM
+ .suspend= sdhci_pltfm_suspend,
+ .resume = sdhci_pltfm_resume,
+#endif
};
-struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
- .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_ADMA,
- /* ADMA has issues. Might be fixable */
- .ops = sdhci_esdhc_ops,
- .init = esdhc_pltfm_init,
- .exit = esdhc_pltfm_exit,
-};
+static int __init sdhci_esdhc_imx_init(void)
+{
+ return platform_driver_register(sdhci_esdhc_imx_driver);
+}
+
+static void __exit sdhci_esdhc_imx_exit(void)
+{
+
Re: [PATCH 5/7] mmc: support sdhci-esdhc-imx as an OF device
[cc'ing linux-mmc to continue this discussion] On Wed, Mar 16, 2011 at 10:39:16PM +0800, Shawn Guo wrote: On Tue, Mar 15, 2011 at 01:59:26PM -0600, Grant Likely wrote: On Mon, Mar 14, 2011 at 10:25:57PM +0800, Shawn Guo wrote: Signed-off-by: Shawn Guo shawn@linaro.org dt support can be added directly to sdchi-pltfm.c drivers now. There is no longer any need to use sdhci-of-core.c any more. For an example, see the patch titled of/tegra: add sdhci device tree handling in my devicetree/test branch. I mentioned this a little bit in the cover letter of the patch set as below. This patch set is to support sdhci-esdhc-imx as an OF device. As there is already powerpc based esdhc OF support, it chose to add OF support for imx esdhc driver in a different way from what sdhci-tegra did. I should read your descriptions more carefully. :-) The tegra approach you made was one of the two options I had, and I happened to love the another more, as it consolidates the eSDHC OF driver for Freescale MPCxxx and i.MX family. Heh, I don't dispute the value of merging code. However, with this approach it means that DT and non-DT imx platforms will be using different drivers for the same device. Given the choices, I'd rather see the imx driver used in both DT and non-DT situations instead of sharing code with the powerpc version. I've learnt the hard way that it is just too painful having two drivers for the same hardware; particularly when the only difference is the method used to probe them. Actually, what I'd *really* rather see is the powerpc code migrated over to sdhci_pltfm.c, and then have the imx compatible value added to it. I'll make sure to get some help from the Freescale powerpc folks to test any patch you produce to that end. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] mmc: consolidate sdhci_pltfm_data and sdhci_of_data into one
[cc'ing linux-mmc@vger.kernel.org]
On Thu, Mar 17, 2011 at 02:33:20PM +0800, Shawn Guo wrote:
On Tue, Mar 15, 2011 at 01:55:13PM -0600, Grant Likely wrote:
On Mon, Mar 14, 2011 at 10:25:56PM +0800, Shawn Guo wrote:
This patch is motivated by the work of supporting sdhci-esdhc-imx as
an OF device. The sdhci-esdhc-imx driver was well designed to be
able to work with either platform or OF bus, with a little effort to
decouple the driver from sdhci_pltfm_data.
Like sdhci_ops works for both platform and OF sdhci driver, the patch
demonstrates that sdhci_pltfm_data and sdhci_of_data can be
consolidated into sdhci_data, which should work for both. As one of
the results, header linux/mmc/sdhci-pltfm.h can be deleted.
Signed-off-by: Shawn Guo shawn@linaro.org
I like the push to unify DT and non-DT usage with the SDHCI drivers,
but I'm not convinced on the approach. Actually, that's more a
comment on the existing code than it is on the this patch.
Yes, this patch is not supposed to mean that much. It only intends
to unify one data structure so that sdhci-esdhc-imx.c can work for
both cases.
The topic you raised here is a bigger one which may involve debate
with authors of both sdhci-pltfm.c and sdhci-of-core.c. We can take
it as the final goal, and this patch could be a first step to that
goal.
I doubt very much that the sdhci-of-core.c users/developers will have
a problem with this. There is a strong trend toward unifying DT and
non-DT code, and Linux has a strong pattern of each driver handling
its own registration.
I actually don't have an objection to this patch specifically, but
rather I want to see the overall direction be to eliminate
sdhci-of-core.c and sdhci-pltfm.c entirely instead of using it more..
On another note, this patch changes a number of names, both of
structures and variables. Specifically:
{sdhci_pltfm_data,sdhci_of_data} == sdhci_data and
pdata == data
However, it would be easier to review if the pdata==data change was
dropped (the name of the local variable doesn't matter that much), and
if sdhci_of_data was renamed to sdhci_pltfm_data. Doing so would make
the diff much smaller without changing the sanity of the resulting
code.
g.
--
Regards,
Shawn
I don't like the way that sdhci-pltfm.c and sdhci-of-core.c each take
responsibility for the .probe hook on each of the implementations.
Doing it that way means that each implementation needs to add a set of
hooks into those core files protected by #ifdefs based on whether or
not the driver is enabled. It also means that loading one driver
means that all the configured sdhci drivers get loaded into the
kernel. This seems backwards.
From what I can see, sdhci-pltfm.c and sdhci-of-core.c both provide
useful common functions that would be used by all sdhci host drivers.
The interface would be a lot cleaner if those routines were exported
for use by other modules, and each driver registered its own probe
hook. It would keep all the driver specific stuff out of
sdhci_pltfm_ids and I think it would result in a cleaner interface
overall.
Here is how I would do it:
1) break the bulk of the sdhci_pltfm.c and sdhci_of_core.c .probe and
.remove hooks out into separate functions; callable by other modules
2) for each driver, add its own probe and remove hooks which calls
into the new functions created in step 1. This would eliminate the
need for the -exit and -init callbacks since they would get rolled
into the new .probe and .remove callbacks
3) finally, when all drivers are removed; eliminate the driver
registration from sdhci_pltfm.c and sdhci_of_core.c because there
wouldn't be any users left.
drivers/mmc/host/sdhci-pxa.c appears to be a good example of how I
think sdhci platform_drivers should be structured.
At the same time, the functionality from sdhci_of_core.c could easily
be migrated into sdhci_pltfm.c.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] of_mmc_spi: add card detect irq support
On Tue, Dec 28, 2010 at 07:05:26PM +0300, Anton Vorontsov wrote: On Mon, Aug 30, 2010 at 11:49:08AM -0600, Grant Likely wrote: On Mon, Aug 30, 2010 at 10:38 AM, David Brownell davi...@pacbell.net wrote: Since I don't do OpenFirmware, let's hear from Grant on this one. Looks good to me. Acked-by: Grant Likely grant.lik...@secretlab.ca I wonder what happened with this patch? Merged, thanks. :-) g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Moving platform_data contents to device tree
On Fri, Feb 11, 2011 at 8:45 AM, Rob Herring robherri...@gmail.com wrote: Thomas, On 02/10/2011 09:29 PM, Thomas Abraham wrote: Hi, I am currently adding device tree support for Samsung's S5PV310 processor. I have a question about handling platform_data when adding device tree support in drivers, specifically about the sdhci-s3c driver. The platform data that is passed to the sdhci-s3c driver is defined in file arch/arm/plat-samsung/plat/sdhci.h, struct s3c_sdhci_platdata. In this structure, there are some function pointers that are passed to the driver. These function pointers are setup by the platform code in arch/arm/plat-samsung. But when platform devices are created from the device tree, how would such function pointers be passed to the driver? Any suggestions on the approach to handle the platform_data information when moving to device tree would be very helpful. As suggested by Grant, you can use bus notifiers. Here is an example: arch/powerpc/platforms/512x/pdm360ng.c Pure data (flags, quirks, chip select assignments, etc.) should ultimately go into the device tree. For board specific functions, use the bus notifiers. For SoC functions, put them in the driver and use the OF match table data pointer. Yes, Rob is right. However, things are simpler if the driver can be designed so that it *doesn't* need platform callbacks. Sometimes it is unavoidable, but in a lot of cases platform callback turn into a way to let platform code twiddle or read GPIOs. In those cases it would be better to turn those callbacks into real gpiolib drivers and design the driver to use the gpio api. See sdhci-of-core.c for an example. On a side note (and not related to sdhci-s3c); I'd like to be rid of both sdhci-of-core.c and sdhci-pltfm.c. I'm all for providing common infrastructure, but I think those two files go about it in the wrong way. Rather than having a single platform_device (well, 2 in this case, but of and non-of can be merged now) that matches against all 'platform' sdhci controllers, each specific controller should have its own platform_driver structure. The way it is done now creates too much indirection (IMHO) and it would be cleaner if the common code was available to the drivers a library function calls. I've put splitting them up onto my todo list if somebody else doesn't beat me to it. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drivers/mmc/host/sdhci-of-core.c on sparc64
On Thu, Jan 13, 2011 at 6:12 PM, Chris Ball c...@laptop.org wrote: Hi, On Thu, Jan 13, 2011 at 03:41:40PM -0800, Andrew Morton wrote: drivers/mmc/host/sdhci-of-core.c:24:25: asm/machdep.h: No such file or directory drivers/mmc/host/sdhci-of-core.c: In function `sdhci_of_wp_inverted': drivers/mmc/host/sdhci-of-core.c:115: error: implicit declaration of function `machine_is' That code's been there for a while. Did someone change Kconfig? Can you attach a .config? asm/machdep.h is arch-specific, so I'd suggest that you're building on an unsupported arch. sparc64 allmodconfig. You're right, Andrew -- Rob (CC'd) changed the MMC Kconfig to build this driver on Sparc. Mainline commit 236cdc7bc71 (of: make drivers depend on CONFIG_OF instead of CONFIG_PPC_OF). Rob also posted a patch to devicetree-discuss@, on top of the one above (mmc: sdhci-of: fix build on non-powerpc platforms), to fix up the Sparc build by ifdef'ing for PPC inside the driver. Grant merged the first patch but not the second, hence sparc64 allmodconfig is broken now. The reason Grant didn't merge the second patch may be that Wolfram objected to #ifdef proliferation inside the driver. Options, as I see it: * revert the commit such that MMC_SDHCI_OF once again depends on PPC_OF * take the second patch as-is * come up with a less-#ifdeffy second patch Wolfram, would appreciate your input on what we should do here. Thanks, I've applied the 2nd patch. I had applied it to my test tree, and indeed I replied saying I did, but I had a corrupt git tree event shortly after applying it to my test branch and evidently lost track of it. It's been pushed out to my next-devicetree branch: git://git.secretlab.ca/git/linux-2.6.git next-devicetree I'll ask Linus to pull after doing some sanity testing. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] mmc: sdhci-of: fix build on non-powerpc platforms
On Tue, Nov 16, 2010 at 04:34:56PM -0600, Rob Herring wrote: On 11/16/2010 03:44 PM, Wolfram Sang wrote: On Tue, Nov 16, 2010 at 02:33:52PM -0600, Rob Herring wrote: From: Rob Herringrob.herr...@calxeda.com Explicitly include err.h, of_address.h and of_irq.h. Make use of machine_is() conditional on PPC. Signed-off-by: Rob Herringrob.herr...@calxeda.com Hmm, sins of the past :/ I wonder if we can get away with less #ifdeffery, will think about it... I don't want to start a long debate, but is updating a kernel without updating the dtb really something to worry about? Yes, once a .dtb is merged we try very hard not to break it. It may need to be updated to enable more features, but the goal is to not regress. One of the reason being that firmware may provide a default, but old, dtb and it is important to still be able to boot on those systems, even if the dtb is immediately going to be updated. That's one of the reasons why it is so important to document and review bindings up front and make sure they make sense before we commit to them. That being said, there are other ways to deal with old dtbs, like fixing up the data at platform setup time. Isn't a year enough of a transition period. No. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc 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/3 v2] sdhci: Add auto CMD12 support for eSDHC driver
On Wed, Aug 4, 2010 at 8:14 PM, Zang Roy-R61911 r61...@freescale.com wrote:
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c6d1bd8..a92566e 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -817,8 +817,12 @@ static void
sdhci_set_transfer_mode(struct sdhci_host *host,
WARN_ON(!host-data);
mode = SDHCI_TRNS_BLK_CNT_EN;
- if (data-blocks 1)
- mode |= SDHCI_TRNS_MULTI;
+ if (data-blocks 1) {
+ if (host-quirks
SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
+ mode |= SDHCI_TRNS_MULTI |
SDHCI_TRNS_ACMD12;
+ else
+ mode |= SDHCI_TRNS_MULTI;
nit:
+ mode |= SDHCI_TRNS_MULTI;
+ if (host-quirks SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
+ mode |= SDHCI_TRNS_ACMD12;
Clearer, no?
why?
Shorter lines, fewer lines, and the SDHCI_TRNS_MULTI is more obviously
unconditional. But as I said, it is a nitpick.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3 v2] dts: Add sdhci,auto-cmd12 field for p4080 device tree
On Mon, Aug 2, 2010 at 9:11 PM, Roy Zang tie-fei.z...@freescale.com wrote: Signed-off-by: Roy Zang tie-fei.z...@freescale.com --- Documentation/powerpc/dts-bindings/fsl/esdhc.txt | 2 ++ arch/powerpc/boot/dts/p4080ds.dts | 1 + 2 files changed, 3 insertions(+), 0 deletions(-) diff --git a/Documentation/powerpc/dts-bindings/fsl/esdhc.txt b/Documentation/powerpc/dts-bindings/fsl/esdhc.txt index 8a00407..64bcb8b 100644 --- a/Documentation/powerpc/dts-bindings/fsl/esdhc.txt +++ b/Documentation/powerpc/dts-bindings/fsl/esdhc.txt @@ -14,6 +14,8 @@ Required properties: reports inverted write-protect state; - sdhci,1-bit-only : (optional) specifies that a controller can only handle 1-bit data transfers. + - sdhci,auto-cmd12: (optional) specifies that a controller can + only handle auto CMD12. I read this, but I still don't understand what it means. What does auto CMD12 mean? Are there other kinds of CMD12? Part of this might be my ignorance about how eSDHC works, but it could be clearer. Also, you can feel free to merge this patch with patch 1 in your series. I makes sense to update the documentation and the driver at the same time. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Request for SPI testing
On Sat, Jul 3, 2010 at 11:05 AM, Antonio Ospite osp...@studenti.unina.it wrote: On Tue, 29 Jun 2010 21:30:34 -0600 Grant Likely grant.lik...@secretlab.ca wrote: I've pushed out Ernst's spi bus locking API patches to the following branch. Before I push them into linux-next, I'd like to get some testing. Can I have some volunteers please to pull this branch and give it a spin? Bonus points for people who have mmc cards multiplexed with other devices on their SPI bus. From a quick test it works for me too, I have both the MMC and the PMIC (battery, regulators) attached to spi, this is Motorola A910 phone (pxa27x). Just FYI I applied the patches to 2.6.34 and tested with this kernel, without the patches I cannot make mmc_spi work reliably (lots of I/O errors, due to conflict with the PMIC) with these patches I can finally use the MMC card. Thanks a lot, Antonio Thanks everyone who tested. I've pushed this out for linux-next inclusion (next-spi branch) g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Request for SPI testing
I've pushed out Ernst's spi bus locking API patches to the following branch. Before I push them into linux-next, I'd like to get some testing. Can I have some volunteers please to pull this branch and give it a spin? Bonus points for people who have mmc cards multiplexed with other devices on their SPI bus. Cheers, g. The following changes since commit 5904b3b81d25166e5e39b9727645bb47937618e3: Linus Torvalds (1): Merge branch 'perf-fixes-for-linus' of git://git.kernel.org/.../tip/linux-2.6-tip are available in the git repository at: git://git.secretlab.ca/git/linux-2.6 spi/test Cory Maccarrone (1): SPI100k: Fix 8-bit and RX-only transfers Ernst Schwab (2): spi/mmc_spi: SPI bus locking API, using mutex spi/mmc_spi: mmc_spi adaptations for SPI bus locking API drivers/mmc/host/mmc_spi.c | 59 ++- drivers/spi/omap_spi_100k.c | 23 +++-- drivers/spi/spi.c | 225 --- include/linux/spi/spi.h | 12 +++ 4 files changed, 227 insertions(+), 92 deletions(-) -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] SDHCI: Don't assign mmc-caps at SDHCI directly
On Mon, Jun 28, 2010 at 11:34 AM, Andrew Morton a...@linux-foundation.org wrote: On Sat, 12 Jun 2010 14:44:50 +0900 Kyungmin Park kyungmin.p...@samsung.com wrote: From: Kyungmin Park kyungmin.p...@samsung.com Some host controller can set mmc-caps before sdhci_add_host. Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/mmc/host/sdhci.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 4321e0c..142419c 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1791,7 +1791,7 @@ int sdhci_add_host(struct sdhci_host *host) else mmc-f_min = host-max_clk / 256; mmc-f_max = host-max_clk; - mmc-caps = MMC_CAP_SDIO_IRQ; + mmc-caps |= MMC_CAP_SDIO_IRQ; if (!(host-quirks SDHCI_QUIRK_FORCE_1_BIT_DATA)) mmc-caps |= MMC_CAP_4_BIT_DATA; A great shower of MMC patches have magically turned up in linux-next, apparently via some tree of Grant's. Those patches changed the above code to look like: if (!(host-quirks SDHCI_QUIRK_NO_SDIO_IRQ)) mmc-caps |= MMC_CAP_SDIO_IRQ; So it appears that this bug is fixed in that code as well. So I'll drop your patch. If the above changes end up not getting merged into mainline then your fix will be lost. That fix was unchangelogged. In fact the patch was completely unchangelogged and I haven't looked at it at all and as far as I can tell none of it has been sent to the mmc list. I messed it up by pushing the wrong branch. I'm pulling it out now and it won't be in the next linux-next. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] ARM: add PrimeCell generic DMA to PL022
On Mon, Mar 29, 2010 at 5:36 PM, Linus Walleij linus.wall...@stericsson.com wrote: This extends the PL022 UART driver with generic DMA engine support using the PrimeCell DMA engine interface. Also fix up the test code for the U300 platform. Signed-off-by: Linus Walleij linus.wall...@stericsson.com --- arch/arm/mach-u300/dummyspichip.c | 1 + drivers/spi/amba-pl022.c | 517 +++-- Hi Linus, I really don't have much to say about this one. It is entirely contained within the amba-pl022 driver, so I don't have any core SPI infrastructure concerns, and it is entirely dependent on the other patches in your series, so it doesn't make sense to merge separately through my SPI tree. I have not tested or reviewed it at all, but I have no objections to you merging it through whichever tree you merge the rest through. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] ARM: PrimeCell DMA patches v4
On Tue, Mar 30, 2010 at 3:57 AM, Linus WALLEIJ linus.wall...@stericsson.com wrote: [Self] This is a fourth iteration of the PrimeCell DMA API on top of the generic DMA devices (sibling to the DMA engine). It's based on the suggestion from Russell to try and define a specific extension subset for DMA devices. Russell Grant can you give some hint on the direction you see for this patch set? The problem we're facing is that next I will start adding DMA support for the U8500 and the MMCI derivate found in that platform doesn't *have* a PIO IRQ, which means the system cannot even boot without some solid DMA framework in place. (It is currently unbootable from the released kernels.) So unless there is some outstanding issue with this approach we pretty much need this now to keep working on mainlining the U8500. I would very much like to have the DMA patches for PrimeCell support pushed through Dan's tree, but that requires your ACKs of course, and it will inevitably collide with other PrimeCell patches for the next merge window (many submitted by myself admittedly). I can feed all the PrimeCell stuff to Dan if all agree that this is a good approach. Another approach is to apply the latest patches from Dan's tree to ARM and SPI alike and then feed the PrimeCell stuff through the ARM tree. I have no objections to the SPI driver getting merged via Dan's tree. g. -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
