In this patch-set we firstly ease the burden on data being passed from
platform data, then start to build up support to finally enable Device
Tree.
Documentation/devicetree/bindings/usb/ux500-usb.txt | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
arch/arm/boot/dts/dbx5x0.dtsi | 31 ++++++++++++++++++++++++++++++-
arch/arm/mach-ux500/cpu-db8500.c | 17 ++---------------
arch/arm/mach-ux500/usb.c | 25 ++++++-------------------
drivers/usb/musb/ux500.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
drivers/usb/musb/ux500_dma.c | 59 +++++++++++++++++++++++++++++++++--------------------------
include/linux/platform_data/usb-musb-ux500.h | 5 +----
7 files changed, 181 insertions(+), 66 deletions(-)
For all ux500 based platforms the maximum number of end-points are used.
Move this knowledge into the driver so we can relinquish the burden from
platform data. This also removes quite a bit of complexity from the driver
and will aid us when we come to enable the driver for Device Tree.
Cc: Felipe Balbi <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/usb.c | 14 ++++++------
drivers/usb/musb/ux500_dma.c | 30 ++++++++------------------
include/linux/platform_data/usb-musb-ux500.h | 5 +----
3 files changed, 16 insertions(+), 33 deletions(-)
diff --git a/arch/arm/mach-ux500/usb.c b/arch/arm/mach-ux500/usb.c
index 821703d..8aff9a4 100644
--- a/arch/arm/mach-ux500/usb.c
+++ b/arch/arm/mach-ux500/usb.c
@@ -22,7 +22,7 @@
.dir = STEDMA40_MEM_TO_PERIPH, \
}
-static struct stedma40_chan_cfg musb_dma_rx_ch[UX500_MUSB_DMA_NUM_RX_CHANNELS]
+static struct stedma40_chan_cfg musb_dma_rx_ch[UX500_MUSB_DMA_NUM_RX_TX_CHANNELS]
= {
MUSB_DMA40_RX_CH,
MUSB_DMA40_RX_CH,
@@ -34,7 +34,7 @@ static struct stedma40_chan_cfg musb_dma_rx_ch[UX500_MUSB_DMA_NUM_RX_CHANNELS]
MUSB_DMA40_RX_CH
};
-static struct stedma40_chan_cfg musb_dma_tx_ch[UX500_MUSB_DMA_NUM_TX_CHANNELS]
+static struct stedma40_chan_cfg musb_dma_tx_ch[UX500_MUSB_DMA_NUM_RX_TX_CHANNELS]
= {
MUSB_DMA40_TX_CH,
MUSB_DMA40_TX_CH,
@@ -46,7 +46,7 @@ static struct stedma40_chan_cfg musb_dma_tx_ch[UX500_MUSB_DMA_NUM_TX_CHANNELS]
MUSB_DMA40_TX_CH,
};
-static void *ux500_dma_rx_param_array[UX500_MUSB_DMA_NUM_RX_CHANNELS] = {
+static void *ux500_dma_rx_param_array[UX500_MUSB_DMA_NUM_RX_TX_CHANNELS] = {
&musb_dma_rx_ch[0],
&musb_dma_rx_ch[1],
&musb_dma_rx_ch[2],
@@ -57,7 +57,7 @@ static void *ux500_dma_rx_param_array[UX500_MUSB_DMA_NUM_RX_CHANNELS] = {
&musb_dma_rx_ch[7]
};
-static void *ux500_dma_tx_param_array[UX500_MUSB_DMA_NUM_TX_CHANNELS] = {
+static void *ux500_dma_tx_param_array[UX500_MUSB_DMA_NUM_RX_TX_CHANNELS] = {
&musb_dma_tx_ch[0],
&musb_dma_tx_ch[1],
&musb_dma_tx_ch[2],
@@ -71,8 +71,6 @@ static void *ux500_dma_tx_param_array[UX500_MUSB_DMA_NUM_TX_CHANNELS] = {
static struct ux500_musb_board_data musb_board_data = {
.dma_rx_param_array = ux500_dma_rx_param_array,
.dma_tx_param_array = ux500_dma_tx_param_array,
- .num_rx_channels = UX500_MUSB_DMA_NUM_RX_CHANNELS,
- .num_tx_channels = UX500_MUSB_DMA_NUM_TX_CHANNELS,
.dma_filter = stedma40_filter,
};
@@ -119,7 +117,7 @@ static inline void ux500_usb_dma_update_rx_ch_config(int *dev_type)
{
u32 idx;
- for (idx = 0; idx < UX500_MUSB_DMA_NUM_RX_CHANNELS; idx++)
+ for (idx = 0; idx < UX500_MUSB_DMA_NUM_RX_TX_CHANNELS; idx++)
musb_dma_rx_ch[idx].dev_type = dev_type[idx];
}
@@ -127,7 +125,7 @@ static inline void ux500_usb_dma_update_tx_ch_config(int *dev_type)
{
u32 idx;
- for (idx = 0; idx < UX500_MUSB_DMA_NUM_TX_CHANNELS; idx++)
+ for (idx = 0; idx < UX500_MUSB_DMA_NUM_RX_TX_CHANNELS; idx++)
musb_dma_tx_ch[idx].dev_type = dev_type[idx];
}
diff --git a/drivers/usb/musb/ux500_dma.c b/drivers/usb/musb/ux500_dma.c
index 039e567d..c75e07a 100644
--- a/drivers/usb/musb/ux500_dma.c
+++ b/drivers/usb/musb/ux500_dma.c
@@ -47,10 +47,8 @@ struct ux500_dma_channel {
struct ux500_dma_controller {
struct dma_controller controller;
- struct ux500_dma_channel rx_channel[UX500_MUSB_DMA_NUM_RX_CHANNELS];
- struct ux500_dma_channel tx_channel[UX500_MUSB_DMA_NUM_TX_CHANNELS];
- u32 num_rx_channels;
- u32 num_tx_channels;
+ struct ux500_dma_channel rx_channel[UX500_MUSB_DMA_NUM_RX_TX_CHANNELS];
+ struct ux500_dma_channel tx_channel[UX500_MUSB_DMA_NUM_RX_TX_CHANNELS];
void *private_data;
dma_addr_t phy_base;
};
@@ -142,19 +140,15 @@ static struct dma_channel *ux500_dma_channel_allocate(struct dma_controller *c,
struct ux500_dma_channel *ux500_channel = NULL;
struct musb *musb = controller->private_data;
u8 ch_num = hw_ep->epnum - 1;
- u32 max_ch;
- /* Max 8 DMA channels (0 - 7). Each DMA channel can only be allocated
+ /* 8 DMA channels (0 - 7). Each DMA channel can only be allocated
* to specified hw_ep. For example DMA channel 0 can only be allocated
* to hw_ep 1 and 9.
*/
if (ch_num > 7)
ch_num -= 8;
- max_ch = is_tx ? controller->num_tx_channels :
- controller->num_rx_channels;
-
- if (ch_num >= max_ch)
+ if (ch_num >= UX500_MUSB_DMA_NUM_RX_TX_CHANNELS)
return NULL;
ux500_channel = is_tx ? &(controller->tx_channel[ch_num]) :
@@ -262,7 +256,7 @@ static int ux500_dma_controller_stop(struct dma_controller *c)
struct dma_channel *channel;
u8 ch_num;
- for (ch_num = 0; ch_num < controller->num_rx_channels; ch_num++) {
+ for (ch_num = 0; ch_num < UX500_MUSB_DMA_NUM_RX_TX_CHANNELS; ch_num++) {
channel = &controller->rx_channel[ch_num].channel;
ux500_channel = channel->private_data;
@@ -272,7 +266,7 @@ static int ux500_dma_controller_stop(struct dma_controller *c)
dma_release_channel(ux500_channel->dma_chan);
}
- for (ch_num = 0; ch_num < controller->num_tx_channels; ch_num++) {
+ for (ch_num = 0; ch_num < UX500_MUSB_DMA_NUM_RX_TX_CHANNELS; ch_num++) {
channel = &controller->tx_channel[ch_num].channel;
ux500_channel = channel->private_data;
@@ -301,26 +295,21 @@ static int ux500_dma_controller_start(struct dma_controller *c)
void **param_array;
struct ux500_dma_channel *channel_array;
- u32 ch_count;
dma_cap_mask_t mask;
- if ((data->num_rx_channels > UX500_MUSB_DMA_NUM_RX_CHANNELS) ||
- (data->num_tx_channels > UX500_MUSB_DMA_NUM_TX_CHANNELS))
- return -EINVAL;
- controller->num_rx_channels = data->num_rx_channels;
- controller->num_tx_channels = data->num_tx_channels;
dma_cap_zero(mask);
dma_cap_set(DMA_SLAVE, mask);
/* Prepare the loop for RX channels */
channel_array = controller->rx_channel;
- ch_count = data->num_rx_channels;
param_array = data->dma_rx_param_array;
for (dir = 0; dir < 2; dir++) {
- for (ch_num = 0; ch_num < ch_count; ch_num++) {
+ for (ch_num = 0;
+ ch_num < UX500_MUSB_DMA_NUM_RX_TX_CHANNELS;
+ ch_num++) {
ux500_channel = &channel_array[ch_num];
ux500_channel->controller = controller;
ux500_channel->ch_num = ch_num;
@@ -348,7 +337,6 @@ static int ux500_dma_controller_start(struct dma_controller *c)
/* Prepare the loop for TX channels */
channel_array = controller->tx_channel;
- ch_count = data->num_tx_channels;
param_array = data->dma_tx_param_array;
is_tx = 1;
}
diff --git a/include/linux/platform_data/usb-musb-ux500.h b/include/linux/platform_data/usb-musb-ux500.h
index 4c1cc50..dd9c83a 100644
--- a/include/linux/platform_data/usb-musb-ux500.h
+++ b/include/linux/platform_data/usb-musb-ux500.h
@@ -9,14 +9,11 @@
#include <linux/dmaengine.h>
-#define UX500_MUSB_DMA_NUM_RX_CHANNELS 8
-#define UX500_MUSB_DMA_NUM_TX_CHANNELS 8
+#define UX500_MUSB_DMA_NUM_RX_TX_CHANNELS 8
struct ux500_musb_board_data {
void **dma_rx_param_array;
void **dma_tx_param_array;
- u32 num_rx_channels;
- u32 num_tx_channels;
bool (*dma_filter)(struct dma_chan *chan, void *filter_param);
};
--
1.7.10.4
The dma_mask will always be the same as the coherent_dma_mask, so let's
cut down on the platform_data burden and set it as such in the driver.
This also saves us from supporting it separately when we come to enable
this driver for Device Tree.
Cc: Felipe Balbi <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/usb.c | 3 ---
drivers/usb/musb/ux500.c | 2 +-
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/arm/mach-ux500/usb.c b/arch/arm/mach-ux500/usb.c
index 4f68a9c..e6c4a05 100644
--- a/arch/arm/mach-ux500/usb.c
+++ b/arch/arm/mach-ux500/usb.c
@@ -74,8 +74,6 @@ static struct ux500_musb_board_data musb_board_data = {
.dma_filter = stedma40_filter,
};
-static u64 ux500_musb_dmamask = DMA_BIT_MASK(32);
-
static struct musb_hdrc_platform_data musb_platform_data = {
.mode = MUSB_OTG,
.board_data = &musb_board_data,
@@ -98,7 +96,6 @@ struct platform_device ux500_musb_device = {
.id = 0,
.dev = {
.platform_data = &musb_platform_data,
- .dma_mask = &ux500_musb_dmamask,
.coherent_dma_mask = DMA_BIT_MASK(32),
},
.num_resources = ARRAY_SIZE(usb_resources),
diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
index 4a8f5c9..7ddb132 100644
--- a/drivers/usb/musb/ux500.c
+++ b/drivers/usb/musb/ux500.c
@@ -122,7 +122,7 @@ static int ux500_probe(struct platform_device *pdev)
}
musb->dev.parent = &pdev->dev;
- musb->dev.dma_mask = pdev->dev.dma_mask;
+ musb->dev.dma_mask = &pdev->dev.coherent_dma_mask;
musb->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
glue->dev = &pdev->dev;
--
1.7.10.4
As promised, now all devices which resided in u8500_of_init_devices()
have been enabled for Device Tree, we can completely remove it.
Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/cpu-db8500.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index 36ef6ed..a2c83c2 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -221,15 +221,6 @@ struct device * __init u8500_init_devices(struct ab8500_platform_data *ab8500)
}
#ifdef CONFIG_MACH_UX500_DT
-
-/* TODO: Once all pieces are DT:ed, remove completely. */
-static struct device * __init u8500_of_init_devices(void)
-{
- struct device *parent = db8500_soc_device_init();
-
- return parent;
-}
-
static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
/* Requires call-back bindings. */
OF_DEV_AUXDATA("arm,cortex-a9-pmu", 0, "arm-pmu", &db8500_pmu_platdata),
@@ -288,7 +279,7 @@ static const struct of_device_id u8500_local_bus_nodes[] = {
static void __init u8500_init_machine(void)
{
- struct device *parent = NULL;
+ struct device *parent = db8500_soc_device_init();
/* Pinmaps must be in place before devices register */
if (of_machine_is_compatible("st-ericsson,mop500"))
@@ -301,9 +292,6 @@ static void __init u8500_init_machine(void)
else if (of_machine_is_compatible("st-ericsson,ccu9540")) {}
/* TODO: Add pinmaps for ccu9540 board. */
- /* TODO: Export SoC, USB, cpu-freq and DMA40 */
- parent = u8500_of_init_devices();
-
/* automatically probe child nodes of db8500 device */
of_platform_populate(NULL, u8500_local_bus_nodes, u8500_auxdata_lookup, parent);
}
--
1.7.10.4
Now the ux500-musb driver has been enabled for Device Tree, there is no
requirement to register it from platform code.
Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/cpu-db8500.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index 5a6b9ae..36ef6ed 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -227,8 +227,6 @@ static struct device * __init u8500_of_init_devices(void)
{
struct device *parent = db8500_soc_device_init();
- db8500_add_usb(parent, usb_db8500_dma_cfg, usb_db8500_dma_cfg);
-
return parent;
}
--
1.7.10.4
The recently DT:ed MUSB driver will require clock-name by device-name
look-up capability, until common clk has is properly supported by the
ux500 platform.
Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/cpu-db8500.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index 3276472..5a6b9ae 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -259,6 +259,7 @@ static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
OF_DEV_AUXDATA("st,nomadik-i2c", 0x80128000, "nmk-i2c.2", NULL),
OF_DEV_AUXDATA("st,nomadik-i2c", 0x80110000, "nmk-i2c.3", NULL),
OF_DEV_AUXDATA("st,nomadik-i2c", 0x8012a000, "nmk-i2c.4", NULL),
+ OF_DEV_AUXDATA("stericsson,db8500-musb", 0xa03e0000, "musb-ux500.0", NULL),
OF_DEV_AUXDATA("stericsson,db8500-prcmu", 0x80157000, "db8500-prcmu",
&db8500_prcmu_pdata),
/* Requires device name bindings. */
--
1.7.10.4
This patch provides all the information to successfully probe() and
correctly configure the ux500-musb device driver for DMA.
Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/dbx5x0.dtsi | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index 86b35a1..f4f371f 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -175,11 +175,40 @@
prcm = <&prcmu>;
};
- usb@a03e0000 {
+ usb_per5@a03e0000 {
compatible = "stericsson,db8500-musb",
"mentor,musb";
reg = <0xa03e0000 0x10000>;
interrupts = <0 23 0x4>;
+ interrupt-names = "mc";
+
+ mode = "otg";
+
+ dmas = <&dma 38 0 0x2>, /* Logical - DevToMem */
+ <&dma 38 0 0x0>, /* Logical - MemToDev */
+ <&dma 37 0 0x2>, /* Logical - DevToMem */
+ <&dma 37 0 0x0>, /* Logical - MemToDev */
+ <&dma 36 0 0x2>, /* Logical - DevToMem */
+ <&dma 36 0 0x0>, /* Logical - MemToDev */
+ <&dma 19 0 0x2>, /* Logical - DevToMem */
+ <&dma 19 0 0x0>, /* Logical - MemToDev */
+ <&dma 18 0 0x2>, /* Logical - DevToMem */
+ <&dma 18 0 0x0>, /* Logical - MemToDev */
+ <&dma 17 0 0x2>, /* Logical - DevToMem */
+ <&dma 17 0 0x0>, /* Logical - MemToDev */
+ <&dma 16 0 0x2>, /* Logical - DevToMem */
+ <&dma 16 0 0x0>, /* Logical - MemToDev */
+ <&dma 39 0 0x2>, /* Logical - DevToMem */
+ <&dma 39 0 0x0>; /* Logical - MemToDev */
+
+ dma-names = "iep_1_9", "oep_1_9",
+ "iep_2_10", "oep_2_10",
+ "iep_3_11", "oep_3_11",
+ "iep_4_12", "oep_4_12",
+ "iep_5_13", "oep_5_13",
+ "iep_6_14", "oep_6_14",
+ "iep_7_15", "oep_7_15",
+ "iep_8", "oep_8";
};
dma: dma-controller@801C0000 {
--
1.7.10.4
This patch will allow ux500-musb to be probed and configured solely from
configuration found in Device Tree.
Cc: Felipe Balbi <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
.../devicetree/bindings/usb/ux500-usb.txt | 49 +++++++++++++++++++
drivers/usb/musb/ux500.c | 51 ++++++++++++++++++++
2 files changed, 100 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/ux500-usb.txt
diff --git a/Documentation/devicetree/bindings/usb/ux500-usb.txt b/Documentation/devicetree/bindings/usb/ux500-usb.txt
new file mode 100644
index 0000000..2844cbc
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ux500-usb.txt
@@ -0,0 +1,49 @@
+Ux500 MUSB
+
+Required properties:
+ - compatible : Should be "stericsson,db8500-musb"
+ - reg : Offset and length of registers
+ - interrupts : Interrupt; mode, number and trigger
+ - mode : One of; "host", "otg", or "peripheral"
+
+Optional properties:
+ - dmas : A list of dma channels;
+ dma-controller, event-line, fixed-channel, flags
+ - dma-names : An ordered list of channel names affiliated to the above
+
+Example:
+
+usb_per5@a03e0000 {
+ compatible = "stericsson,db8500-musb", "mentor,musb";
+ reg = <0xa03e0000 0x10000>;
+ interrupts = <0 23 0x4>;
+ interrupt-names = "mc";
+
+ mode = "otg";
+
+ dmas = <&dma 38 0 0x2>, /* Logical - DevToMem */
+ <&dma 38 0 0x0>, /* Logical - MemToDev */
+ <&dma 37 0 0x2>, /* Logical - DevToMem */
+ <&dma 37 0 0x0>, /* Logical - MemToDev */
+ <&dma 36 0 0x2>, /* Logical - DevToMem */
+ <&dma 36 0 0x0>, /* Logical - MemToDev */
+ <&dma 19 0 0x2>, /* Logical - DevToMem */
+ <&dma 19 0 0x0>, /* Logical - MemToDev */
+ <&dma 18 0 0x2>, /* Logical - DevToMem */
+ <&dma 18 0 0x0>, /* Logical - MemToDev */
+ <&dma 17 0 0x2>, /* Logical - DevToMem */
+ <&dma 17 0 0x0>, /* Logical - MemToDev */
+ <&dma 16 0 0x2>, /* Logical - DevToMem */
+ <&dma 16 0 0x0>, /* Logical - MemToDev */
+ <&dma 39 0 0x2>, /* Logical - DevToMem */
+ <&dma 39 0 0x0>; /* Logical - MemToDev */
+
+ dma-names = "iep_1_9", "oep_1_9",
+ "iep_2_10", "oep_2_10",
+ "iep_3_11", "oep_3_11",
+ "iep_4_12", "oep_4_12",
+ "iep_5_13", "oep_5_13",
+ "iep_6_14", "oep_6_14",
+ "iep_7_15", "oep_7_15",
+ "iep_8", "oep_8";
+};
diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
index 7ddb132..6418624 100644
--- a/drivers/usb/musb/ux500.c
+++ b/drivers/usb/musb/ux500.c
@@ -25,6 +25,7 @@
#include <linux/clk.h>
#include <linux/err.h>
#include <linux/io.h>
+#include <linux/of.h>
#include <linux/platform_device.h>
#include "musb_core.h"
@@ -88,14 +89,57 @@ static const struct musb_platform_ops ux500_ops = {
.exit = ux500_musb_exit,
};
+static struct musb_hdrc_platform_data *
+ux500_of_probe(struct platform_device *pdev, struct device_node *np)
+{
+ struct musb_hdrc_platform_data *pdata;
+ const char *mode;
+ int strlen;
+
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return NULL;
+
+ mode = of_get_property(np, "mode", &strlen);
+ if (!mode) {
+ dev_err(&pdev->dev, "No 'mode' property found\n");
+ return NULL;
+ }
+
+ if (strlen > 0) {
+ if (!strcmp(mode, "host"))
+ pdata->mode = MUSB_HOST;
+ if (!strcmp(mode, "otg"))
+ pdata->mode = MUSB_OTG;
+ if (!strcmp(mode, "peripheral"))
+ pdata->mode = MUSB_PERIPHERAL;
+ }
+
+ return pdata;
+}
+
static int ux500_probe(struct platform_device *pdev)
{
struct musb_hdrc_platform_data *pdata = pdev->dev.platform_data;
+ struct device_node *np = pdev->dev.of_node;
struct platform_device *musb;
struct ux500_glue *glue;
struct clk *clk;
int ret = -ENOMEM;
+ if (!pdata) {
+ if (np) {
+ pdata = ux500_of_probe(pdev, np);
+ if (!pdata)
+ goto err0;
+
+ pdev->dev.platform_data = pdata;
+ } else {
+ dev_err(&pdev->dev, "no pdata or device tree found\n");
+ goto err0;
+ }
+ }
+
glue = kzalloc(sizeof(*glue), GFP_KERNEL);
if (!glue) {
dev_err(&pdev->dev, "failed to allocate glue context\n");
@@ -124,6 +168,7 @@ static int ux500_probe(struct platform_device *pdev)
musb->dev.parent = &pdev->dev;
musb->dev.dma_mask = &pdev->dev.coherent_dma_mask;
musb->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
+ musb->dev.of_node = pdev->dev.of_node;
glue->dev = &pdev->dev;
glue->musb = musb;
@@ -222,12 +267,18 @@ static const struct dev_pm_ops ux500_pm_ops = {
#define DEV_PM_OPS NULL
#endif
+static const struct of_device_id ux500_match[] = {
+ { .compatible = "stericsson,db8500-musb", },
+ {}
+};
+
static struct platform_driver ux500_driver = {
.probe = ux500_probe,
.remove = ux500_remove,
.driver = {
.name = "musb-ux500",
.pm = DEV_PM_OPS,
+ .of_match_table = ux500_match,
},
};
--
1.7.10.4
In its current state, the ux500-musb driver uses platform data pointers
blindly with no prior checking. If no platform data pointer is passed
this will Oops the kernel. In this patch we ensure platform data and
board data are present prior to using them.
Cc: Felipe Balbi <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
drivers/usb/musb/ux500_dma.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/musb/ux500_dma.c b/drivers/usb/musb/ux500_dma.c
index c75e07a..02b4a6e 100644
--- a/drivers/usb/musb/ux500_dma.c
+++ b/drivers/usb/musb/ux500_dma.c
@@ -287,7 +287,7 @@ static int ux500_dma_controller_start(struct dma_controller *c)
struct musb *musb = controller->private_data;
struct device *dev = musb->controller;
struct musb_hdrc_platform_data *plat = dev->platform_data;
- struct ux500_musb_board_data *data = plat->board_data;
+ struct ux500_musb_board_data *data;
struct dma_channel *dma_channel = NULL;
u32 ch_num;
u8 dir;
@@ -297,14 +297,19 @@ static int ux500_dma_controller_start(struct dma_controller *c)
struct ux500_dma_channel *channel_array;
dma_cap_mask_t mask;
+ if (!plat) {
+ dev_err(musb->controller, "No platform data\n");
+ return -EINVAL;
+ }
+ data = plat->board_data;
dma_cap_zero(mask);
dma_cap_set(DMA_SLAVE, mask);
/* Prepare the loop for RX channels */
channel_array = controller->rx_channel;
- param_array = data->dma_rx_param_array;
+ param_array = (data) ? data->dma_rx_param_array : NULL;
for (dir = 0; dir < 2; dir++) {
for (ch_num = 0;
@@ -337,7 +342,7 @@ static int ux500_dma_controller_start(struct dma_controller *c)
/* Prepare the loop for TX channels */
channel_array = controller->tx_channel;
- param_array = data->dma_tx_param_array;
+ param_array = (data) ? data->dma_tx_param_array : NULL;
is_tx = 1;
}
--
1.7.10.4
If we can ever get to a state where we can solely search for DMA channels
by name, this will almost completely alleviate the requirement to pass
copious amounts of information though platform data. Here we take the
first step towards this. The next step will be to enable Device Tree
complete with name<->event_line mapping.
Cc: Felipe Balbi <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
drivers/usb/musb/ux500_dma.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/musb/ux500_dma.c b/drivers/usb/musb/ux500_dma.c
index 02b4a6e..6eef449 100644
--- a/drivers/usb/musb/ux500_dma.c
+++ b/drivers/usb/musb/ux500_dma.c
@@ -33,6 +33,11 @@
#include <linux/platform_data/usb-musb-ux500.h>
#include "musb_core.h"
+static const char *iep_chan_names[] = { "iep_1_9", "iep_2_10", "iep_3_11", "iep_4_12",
+ "iep_5_13", "iep_6_14", "iep_7_15", "iep_8" };
+static const char *oep_chan_names[] = { "oep_1_9", "oep_2_10", "oep_3_11", "oep_4_12",
+ "oep_5_13", "oep_6_14", "oep_7_15", "oep_8" };
+
struct ux500_dma_channel {
struct dma_channel channel;
struct ux500_dma_controller *controller;
@@ -289,6 +294,7 @@ static int ux500_dma_controller_start(struct dma_controller *c)
struct musb_hdrc_platform_data *plat = dev->platform_data;
struct ux500_musb_board_data *data;
struct dma_channel *dma_channel = NULL;
+ char **chan_names;
u32 ch_num;
u8 dir;
u8 is_tx = 0;
@@ -310,6 +316,7 @@ static int ux500_dma_controller_start(struct dma_controller *c)
/* Prepare the loop for RX channels */
channel_array = controller->rx_channel;
param_array = (data) ? data->dma_rx_param_array : NULL;
+ chan_names = (char **)iep_chan_names;
for (dir = 0; dir < 2; dir++) {
for (ch_num = 0;
@@ -325,9 +332,15 @@ static int ux500_dma_controller_start(struct dma_controller *c)
dma_channel->status = MUSB_DMA_STATUS_FREE;
dma_channel->max_len = SZ_16M;
- ux500_channel->dma_chan = dma_request_channel(mask,
- data->dma_filter,
- param_array[ch_num]);
+ ux500_channel->dma_chan =
+ dma_request_slave_channel(dev, chan_names[ch_num]);
+
+ if (!ux500_channel->dma_chan)
+ ux500_channel->dma_chan =
+ dma_request_channel(mask,
+ data->dma_filter,
+ param_array[ch_num]);
+
if (!ux500_channel->dma_chan) {
ERR("Dma pipe allocation error dir=%d ch=%d\n",
dir, ch_num);
@@ -343,6 +356,7 @@ static int ux500_dma_controller_start(struct dma_controller *c)
/* Prepare the loop for TX channels */
channel_array = controller->tx_channel;
param_array = (data) ? data->dma_tx_param_array : NULL;
+ chan_names = (char **)oep_chan_names;
is_tx = 1;
}
--
1.7.10.4
The MUSB HDRC configuration never changes between each of the ux500
supported platforms, so there's little point passing it though platform
data. If we set it in the driver instead, we can make good use of it
when booting with either ATAGs or Device Tree.
Cc: Felipe Balbi <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/usb.c | 8 --------
drivers/usb/musb/ux500.c | 8 ++++++++
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-ux500/usb.c b/arch/arm/mach-ux500/usb.c
index 8aff9a4..4f68a9c 100644
--- a/arch/arm/mach-ux500/usb.c
+++ b/arch/arm/mach-ux500/usb.c
@@ -76,16 +76,8 @@ static struct ux500_musb_board_data musb_board_data = {
static u64 ux500_musb_dmamask = DMA_BIT_MASK(32);
-static struct musb_hdrc_config musb_hdrc_config = {
- .multipoint = true,
- .dyn_fifo = true,
- .num_eps = 16,
- .ram_bits = 16,
-};
-
static struct musb_hdrc_platform_data musb_platform_data = {
.mode = MUSB_OTG,
- .config = &musb_hdrc_config,
.board_data = &musb_board_data,
};
diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
index 13a3929..4a8f5c9 100644
--- a/drivers/usb/musb/ux500.c
+++ b/drivers/usb/musb/ux500.c
@@ -29,6 +29,13 @@
#include "musb_core.h"
+static struct musb_hdrc_config ux500_musb_hdrc_config = {
+ .multipoint = true,
+ .dyn_fifo = true,
+ .num_eps = 16,
+ .ram_bits = 16,
+};
+
struct ux500_glue {
struct device *dev;
struct platform_device *musb;
@@ -123,6 +130,7 @@ static int ux500_probe(struct platform_device *pdev)
glue->clk = clk;
pdata->platform_ops = &ux500_ops;
+ pdata->config = &ux500_musb_hdrc_config;
platform_set_drvdata(pdev, glue);
--
1.7.10.4
On Tuesday 23 April 2013, Lee Jones wrote:
> +Required properties:
> + - compatible : Should be "stericsson,db8500-musb"
> + - reg : Offset and length of registers
> + - interrupts : Interrupt; mode, number and trigger
> + - mode : One of; "host", "otg", or "peripheral"
I think we don't have any precedent for this combination of "mode" property
and contents. You could either try to match ehci-omap.txt and do
- mode : Should be "3" to represent OTG. "1" signifies HOST and "2"
represents PERIPHERAL.
Or you match fsl-usb.txt and nvidia,tegra20-ehci.txt, defining
- dr_mode : dual role mode. Indicates the working mode to
controllers. Can be "host", "peripheral", or "otg". Default to
"host" if not defined for backward compatibility.
> +Optional properties:
> + - dmas : A list of dma channels;
> + dma-controller, event-line, fixed-channel, flags
> + - dma-names : An ordered list of channel names affiliated to the above
Please list the exact dma-names that are required.
Arnd
On Tue, Apr 23, 2013 at 05:14:06PM +0200, Arnd Bergmann wrote:
> On Tuesday 23 April 2013, Lee Jones wrote:
> > +Required properties:
> > + - compatible : Should be "stericsson,db8500-musb"
> > + - reg : Offset and length of registers
> > + - interrupts : Interrupt; mode, number and trigger
> > + - mode : One of; "host", "otg", or "peripheral"
>
> I think we don't have any precedent for this combination of "mode" property
> and contents. You could either try to match ehci-omap.txt and do
there is dr_mode which other folks are using. Let's use the same thing.
--
balbi
On Tuesday 23 April 2013, Lee Jones wrote:
> In this patch-set we firstly ease the burden on data being passed from
> platform data, then start to build up support to finally enable Device
> Tree.
Very nice series!
I only have two small comments on the binding document, everything else looks
great.
Acked-by: Arnd Bergmann <[email protected]>
On Tuesday 23 April 2013, Felipe Balbi wrote:
> On Tue, Apr 23, 2013 at 05:14:06PM +0200, Arnd Bergmann wrote:
> > On Tuesday 23 April 2013, Lee Jones wrote:
> > > +Required properties:
> > > + - compatible : Should be "stericsson,db8500-musb"
> > > + - reg : Offset and length of registers
> > > + - interrupts : Interrupt; mode, number and trigger
> > > + - mode : One of; "host", "otg", or "peripheral"
> >
> > I think we don't have any precedent for this combination of "mode" property
> > and contents. You could either try to match ehci-omap.txt and do
>
> there is dr_mode which other folks are using. Let's use the same thing.
On that topic, should we support the same property in am35x.c and
musbhsdma.c? They currently use an integer rather than a string.
Arnd
Hi,
On Tue, Apr 23, 2013 at 05:27:10PM +0200, Arnd Bergmann wrote:
> > > > +Required properties:
> > > > + - compatible : Should be "stericsson,db8500-musb"
> > > > + - reg : Offset and length of registers
> > > > + - interrupts : Interrupt; mode, number and trigger
> > > > + - mode : One of; "host", "otg", or "peripheral"
> > >
> > > I think we don't have any precedent for this combination of "mode" property
> > > and contents. You could either try to match ehci-omap.txt and do
> >
> > there is dr_mode which other folks are using. Let's use the same thing.
>
> On that topic, should we support the same property in am35x.c and
> musbhsdma.c? They currently use an integer rather than a string.
certainly :) I also plan to add dr_mode support for dwc3.
--
balbi
Hello.
On 04/23/2013 07:03 PM, Lee Jones wrote:
> In its current state, the ux500-musb driver uses platform data pointers
> blindly with no prior checking. If no platform data pointer is passed
> this will Oops the kernel. In this patch we ensure platform data and
> board data are present prior to using them.
>
> Cc: Felipe Balbi <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/usb/musb/ux500_dma.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/musb/ux500_dma.c b/drivers/usb/musb/ux500_dma.c
> index c75e07a..02b4a6e 100644
> --- a/drivers/usb/musb/ux500_dma.c
> +++ b/drivers/usb/musb/ux500_dma.c
> @@ -287,7 +287,7 @@ static int ux500_dma_controller_start(struct dma_controller *c)
> struct musb *musb = controller->private_data;
> struct device *dev = musb->controller;
> struct musb_hdrc_platform_data *plat = dev->platform_data;
> - struct ux500_musb_board_data *data = plat->board_data;
> + struct ux500_musb_board_data *data;
> struct dma_channel *dma_channel = NULL;
> u32 ch_num;
> u8 dir;
> @@ -297,14 +297,19 @@ static int ux500_dma_controller_start(struct dma_controller *c)
[...]
>
> /* Prepare the loop for RX channels */
> channel_array = controller->rx_channel;
> - param_array = data->dma_rx_param_array;
> + param_array = (data) ? data->dma_rx_param_array : NULL;
Why enclose a simple variable in parens?
>
> for (dir = 0; dir < 2; dir++) {
> for (ch_num = 0;
> @@ -337,7 +342,7 @@ static int ux500_dma_controller_start(struct dma_controller *c)
>
> /* Prepare the loop for TX channels */
> channel_array = controller->tx_channel;
> - param_array = data->dma_tx_param_array;
> + param_array = (data) ? data->dma_tx_param_array : NULL;
Again, why?
WBR, Sergei
Hi Sergei,
> > struct musb_hdrc_platform_data *plat = dev->platform_data;
> >- struct ux500_musb_board_data *data = plat->board_data;
> >+ struct ux500_musb_board_data *data;
> >- param_array = data->dma_rx_param_array;
> >+ param_array = (data) ? data->dma_rx_param_array : NULL;
>
> Why enclose a simple variable in parens?
Because 'data' is a pointer, so it contains a memory location, but if
'plat->board_data' is NULL, then 'data' will be NULL (essentially
memory location 0x00000000).
So if we were to read-in to 'struct ux500_musb_board_data *data', by
index 'dma_rx_param_array', which I believe is '0' in this case:
struct ux500_musb_board_data {
void **dma_rx_param_array;
void **dma_tx_param_array;
bool (*dma_filter)(struct dma_chan *chan, void *filter_param);
};
... then we're saying take the data from this memory location:
param_array = *((0x00000000)->(0x0));
Which will cause a kernel Oops, due to the fact that address 0x0 isn't
allocated to us, so you get something like:
"Unable to handle kernel NULL pointer dereference at virtual address 00000000"
Hope that helps.
Kind regards,
Lee
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
This patch provides all the information to successfully probe() and
correctly configure the ux500-musb device driver for DMA.
Signed-off-by: Lee Jones <[email protected]>
diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index 86b35a1..838214c 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -175,11 +175,40 @@
prcm = <&prcmu>;
};
- usb@a03e0000 {
+ usb_per5@a03e0000 {
compatible = "stericsson,db8500-musb",
"mentor,musb";
reg = <0xa03e0000 0x10000>;
interrupts = <0 23 0x4>;
+ interrupt-names = "mc";
+
+ dr_mode = "otg";
+
+ dmas = <&dma 38 0 0x2>, /* Logical - DevToMem */
+ <&dma 38 0 0x0>, /* Logical - MemToDev */
+ <&dma 37 0 0x2>, /* Logical - DevToMem */
+ <&dma 37 0 0x0>, /* Logical - MemToDev */
+ <&dma 36 0 0x2>, /* Logical - DevToMem */
+ <&dma 36 0 0x0>, /* Logical - MemToDev */
+ <&dma 19 0 0x2>, /* Logical - DevToMem */
+ <&dma 19 0 0x0>, /* Logical - MemToDev */
+ <&dma 18 0 0x2>, /* Logical - DevToMem */
+ <&dma 18 0 0x0>, /* Logical - MemToDev */
+ <&dma 17 0 0x2>, /* Logical - DevToMem */
+ <&dma 17 0 0x0>, /* Logical - MemToDev */
+ <&dma 16 0 0x2>, /* Logical - DevToMem */
+ <&dma 16 0 0x0>, /* Logical - MemToDev */
+ <&dma 39 0 0x2>, /* Logical - DevToMem */
+ <&dma 39 0 0x0>; /* Logical - MemToDev */
+
+ dma-names = "iep_1_9", "oep_1_9",
+ "iep_2_10", "oep_2_10",
+ "iep_3_11", "oep_3_11",
+ "iep_4_12", "oep_4_12",
+ "iep_5_13", "oep_5_13",
+ "iep_6_14", "oep_6_14",
+ "iep_7_15", "oep_7_15",
+ "iep_8", "oep_8";
};
dma: dma-controller@801C0000 {
usb: musb: ux500: add device tree probing support
This patch will allow ux500-musb to be probed and configured solely from
configuration found in Device Tree.
Cc: Felipe Balbi <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
diff --git a/Documentation/devicetree/bindings/usb/ux500-usb.txt b/Documentation/devicetree/bindings/usb/ux500-usb.txt
new file mode 100644
index 0000000..330d6ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ux500-usb.txt
@@ -0,0 +1,50 @@
+Ux500 MUSB
+
+Required properties:
+ - compatible : Should be "stericsson,db8500-musb"
+ - reg : Offset and length of registers
+ - interrupts : Interrupt; mode, number and trigger
+ - dr_mode : Dual-role; either host mode "host", peripheral mode "peripheral"
+ or both "otg"
+
+Optional properties:
+ - dmas : A list of dma channels;
+ dma-controller, event-line, fixed-channel, flags
+ - dma-names : An ordered list of channel names affiliated to the above
+
+Example:
+
+usb_per5@a03e0000 {
+ compatible = "stericsson,db8500-musb", "mentor,musb";
+ reg = <0xa03e0000 0x10000>;
+ interrupts = <0 23 0x4>;
+ interrupt-names = "mc";
+
+ dr_mode = "otg";
+
+ dmas = <&dma 38 0 0x2>, /* Logical - DevToMem */
+ <&dma 38 0 0x0>, /* Logical - MemToDev */
+ <&dma 37 0 0x2>, /* Logical - DevToMem */
+ <&dma 37 0 0x0>, /* Logical - MemToDev */
+ <&dma 36 0 0x2>, /* Logical - DevToMem */
+ <&dma 36 0 0x0>, /* Logical - MemToDev */
+ <&dma 19 0 0x2>, /* Logical - DevToMem */
+ <&dma 19 0 0x0>, /* Logical - MemToDev */
+ <&dma 18 0 0x2>, /* Logical - DevToMem */
+ <&dma 18 0 0x0>, /* Logical - MemToDev */
+ <&dma 17 0 0x2>, /* Logical - DevToMem */
+ <&dma 17 0 0x0>, /* Logical - MemToDev */
+ <&dma 16 0 0x2>, /* Logical - DevToMem */
+ <&dma 16 0 0x0>, /* Logical - MemToDev */
+ <&dma 39 0 0x2>, /* Logical - DevToMem */
+ <&dma 39 0 0x0>; /* Logical - MemToDev */
+
+ dma-names = "iep_1_9", "oep_1_9",
+ "iep_2_10", "oep_2_10",
+ "iep_3_11", "oep_3_11",
+ "iep_4_12", "oep_4_12",
+ "iep_5_13", "oep_5_13",
+ "iep_6_14", "oep_6_14",
+ "iep_7_15", "oep_7_15",
+ "iep_8", "oep_8";
+};
diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
index 7ddb132..5ff304c 100644
--- a/drivers/usb/musb/ux500.c
+++ b/drivers/usb/musb/ux500.c
@@ -25,6 +25,7 @@
#include <linux/clk.h>
#include <linux/err.h>
#include <linux/io.h>
+#include <linux/of.h>
#include <linux/platform_device.h>
#include "musb_core.h"
@@ -88,14 +89,57 @@ static const struct musb_platform_ops ux500_ops = {
.exit = ux500_musb_exit,
};
+static struct musb_hdrc_platform_data *
+ux500_of_probe(struct platform_device *pdev, struct device_node *np)
+{
+ struct musb_hdrc_platform_data *pdata;
+ const char *mode;
+ int strlen;
+
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return NULL;
+
+ mode = of_get_property(np, "dr_mode", &strlen);
+ if (!mode) {
+ dev_err(&pdev->dev, "No 'dr_mode' property found\n");
+ return NULL;
+ }
+
+ if (strlen > 0) {
+ if (!strcmp(mode, "host"))
+ pdata->mode = MUSB_HOST;
+ if (!strcmp(mode, "otg"))
+ pdata->mode = MUSB_OTG;
+ if (!strcmp(mode, "peripheral"))
+ pdata->mode = MUSB_PERIPHERAL;
+ }
+
+ return pdata;
+}
+
static int ux500_probe(struct platform_device *pdev)
{
struct musb_hdrc_platform_data *pdata = pdev->dev.platform_data;
+ struct device_node *np = pdev->dev.of_node;
struct platform_device *musb;
struct ux500_glue *glue;
struct clk *clk;
int ret = -ENOMEM;
+ if (!pdata) {
+ if (np) {
+ pdata = ux500_of_probe(pdev, np);
+ if (!pdata)
+ goto err0;
+
+ pdev->dev.platform_data = pdata;
+ } else {
+ dev_err(&pdev->dev, "no pdata or device tree found\n");
+ goto err0;
+ }
+ }
+
glue = kzalloc(sizeof(*glue), GFP_KERNEL);
if (!glue) {
dev_err(&pdev->dev, "failed to allocate glue context\n");
@@ -124,6 +168,7 @@ static int ux500_probe(struct platform_device *pdev)
musb->dev.parent = &pdev->dev;
musb->dev.dma_mask = &pdev->dev.coherent_dma_mask;
musb->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
+ musb->dev.of_node = pdev->dev.of_node;
glue->dev = &pdev->dev;
glue->musb = musb;
@@ -222,12 +267,18 @@ static const struct dev_pm_ops ux500_pm_ops = {
#define DEV_PM_OPS NULL
#endif
+static const struct of_device_id ux500_match[] = {
+ { .compatible = "stericsson,db8500-musb", },
+ {}
+};
+
static struct platform_driver ux500_driver = {
.probe = ux500_probe,
.remove = ux500_remove,
.driver = {
.name = "musb-ux500",
.pm = DEV_PM_OPS,
+ .of_match_table = ux500_match,
},
};
Hello.
On 24-04-2013 10:53, Lee Jones wrote:
>>> struct musb_hdrc_platform_data *plat = dev->platform_data;
>>> - struct ux500_musb_board_data *data = plat->board_data;
>>> + struct ux500_musb_board_data *data;
>>> - param_array = data->dma_rx_param_array;
>>> + param_array = (data) ? data->dma_rx_param_array : NULL;
>> Why enclose a simple variable in parens?
> Because 'data' is a pointer, so it contains a memory location,
Pointer points to memory location, not contains it.
> but if
> 'plat->board_data' is NULL, then 'data' will be NULL (essentially
> memory location 0x00000000).
So what?
> So if we were to read-in to 'struct ux500_musb_board_data *data', by
> index 'dma_rx_param_array', which I believe is '0' in this case:
> struct ux500_musb_board_data {
> void **dma_rx_param_array;
> void **dma_tx_param_array;
> bool (*dma_filter)(struct dma_chan *chan, void *filter_param);
> };
> ... then we're saying take the data from this memory location:
> param_array = *((0x00000000)->(0x0));
> Which will cause a kernel Oops, due to the fact that address 0x0 isn't
> allocated to us, so you get something like:
> "Unable to handle kernel NULL pointer dereference at virtual address 00000000"
We're not dereferencing 'data', so I completely fail to follow you.
> Hope that helps.
Not at all.
> Kind regards,
> Lee
WBR, Sergei
On Wed, Apr 24, 2013 at 06:00:28PM +0400, Sergei Shtylyov wrote:
> Hello.
>
> On 24-04-2013 10:53, Lee Jones wrote:
>
> >>> struct musb_hdrc_platform_data *plat = dev->platform_data;
> >>>- struct ux500_musb_board_data *data = plat->board_data;
> >>>+ struct ux500_musb_board_data *data;
>
> >>>- param_array = data->dma_rx_param_array;
> >>>+ param_array = (data) ? data->dma_rx_param_array : NULL;
>
> >> Why enclose a simple variable in parens?
>
> >Because 'data' is a pointer, so it contains a memory location,
heh, I don't think you fully understood Sergei's comment. He's asking
why you used:
param_array = (data) ? data->dma_rx_param_array : NULL;
instead of:
param_array = data ? data->dma_rx_param_array : NULL;
He's saying, correctly so, that the parens around data is unnecessary.
--
balbi
On Wednesday 24 April 2013, Lee Jones wrote:
> Yeah, I agree, although does it make a difference?
>
> Is it evaluated a second time, or does it take up extra cycles by being
> enclosed in parentheses?
>
> Or is this just a coding style thing?
Just coding style. I agree you should have no parentheses there, but it
does not change the compiled code.
Arnd
In its current state, the ux500-musb driver uses platform data pointers
blindly with no prior checking. If no platform data pointer is passed
this will Oops the kernel. In this patch we ensure platform data and
board data are present prior to using them.
Cc: Felipe Balbi <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
diff --git a/drivers/usb/musb/ux500_dma.c b/drivers/usb/musb/ux500_dma.c
index c75e07a..8d5128d 100644
--- a/drivers/usb/musb/ux500_dma.c
+++ b/drivers/usb/musb/ux500_dma.c
@@ -287,7 +287,7 @@ static int ux500_dma_controller_start(struct dma_controller *c)
struct musb *musb = controller->private_data;
struct device *dev = musb->controller;
struct musb_hdrc_platform_data *plat = dev->platform_data;
- struct ux500_musb_board_data *data = plat->board_data;
+ struct ux500_musb_board_data *data;
struct dma_channel *dma_channel = NULL;
u32 ch_num;
u8 dir;
@@ -297,14 +297,19 @@ static int ux500_dma_controller_start(struct dma_controller *c)
struct ux500_dma_channel *channel_array;
dma_cap_mask_t mask;
+ if (!plat) {
+ dev_err(musb->controller, "No platform data\n");
+ return -EINVAL;
+ }
+ data = plat->board_data;
dma_cap_zero(mask);
dma_cap_set(DMA_SLAVE, mask);
/* Prepare the loop for RX channels */
channel_array = controller->rx_channel;
- param_array = data->dma_rx_param_array;
+ param_array = data ? data->dma_rx_param_array : NULL;
for (dir = 0; dir < 2; dir++) {
for (ch_num = 0;
@@ -337,7 +342,7 @@ static int ux500_dma_controller_start(struct dma_controller *c)
/* Prepare the loop for TX channels */
channel_array = controller->tx_channel;
- param_array = data->dma_tx_param_array;
+ param_array = data ? data->dma_tx_param_array : NULL;
is_tx = 1;
}
If we can ever get to a state where we can solely search for DMA channels
by name, this will almost completely alleviate the requirement to pass
copious amounts of information though platform data. Here we take the
first step towards this. The next step will be to enable Device Tree
complete with name<->event_line mapping.
Cc: Felipe Balbi <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
diff --git a/drivers/usb/musb/ux500_dma.c b/drivers/usb/musb/ux500_dma.c
index 8d5128d..99ad1fe 100644
--- a/drivers/usb/musb/ux500_dma.c
+++ b/drivers/usb/musb/ux500_dma.c
@@ -33,6 +33,11 @@
#include <linux/platform_data/usb-musb-ux500.h>
#include "musb_core.h"
+static const char *iep_chan_names[] = { "iep_1_9", "iep_2_10", "iep_3_11", "iep_4_12",
+ "iep_5_13", "iep_6_14", "iep_7_15", "iep_8" };
+static const char *oep_chan_names[] = { "oep_1_9", "oep_2_10", "oep_3_11", "oep_4_12",
+ "oep_5_13", "oep_6_14", "oep_7_15", "oep_8" };
+
struct ux500_dma_channel {
struct dma_channel channel;
struct ux500_dma_controller *controller;
@@ -289,6 +294,7 @@ static int ux500_dma_controller_start(struct dma_controller *c)
struct musb_hdrc_platform_data *plat = dev->platform_data;
struct ux500_musb_board_data *data;
struct dma_channel *dma_channel = NULL;
+ char **chan_names;
u32 ch_num;
u8 dir;
u8 is_tx = 0;
@@ -310,6 +316,7 @@ static int ux500_dma_controller_start(struct dma_controller *c)
/* Prepare the loop for RX channels */
channel_array = controller->rx_channel;
param_array = data ? data->dma_rx_param_array : NULL;
+ chan_names = (char **)iep_chan_names;
for (dir = 0; dir < 2; dir++) {
for (ch_num = 0;
@@ -325,9 +332,15 @@ static int ux500_dma_controller_start(struct dma_controller *c)
dma_channel->status = MUSB_DMA_STATUS_FREE;
dma_channel->max_len = SZ_16M;
- ux500_channel->dma_chan = dma_request_channel(mask,
- data->dma_filter,
- param_array[ch_num]);
+ ux500_channel->dma_chan =
+ dma_request_slave_channel(dev, chan_names[ch_num]);
+
+ if (!ux500_channel->dma_chan)
+ ux500_channel->dma_chan =
+ dma_request_channel(mask,
+ data->dma_filter,
+ param_array[ch_num]);
+
if (!ux500_channel->dma_chan) {
ERR("Dma pipe allocation error dir=%d ch=%d\n",
dir, ch_num);
@@ -343,6 +356,7 @@ static int ux500_dma_controller_start(struct dma_controller *c)
/* Prepare the loop for TX channels */
channel_array = controller->tx_channel;
param_array = data ? data->dma_tx_param_array : NULL;
+ chan_names = (char **)oep_chan_names;
is_tx = 1;
}
Now that Fabio has written a few patches to this driver I'd like his help
in reviewing this series, can you make sure he gets a copy? Also
please include Praveena and Mian, thanks.
On Tue, Apr 23, 2013 at 5:03 PM, Lee Jones <[email protected]> wrote:
> For all ux500 based platforms the maximum number of end-points are used.
> Move this knowledge into the driver so we can relinquish the burden from
> platform data. This also removes quite a bit of complexity from the driver
> and will aid us when we come to enable the driver for Device Tree.
>
> Cc: Felipe Balbi <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
Looks good to me.
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Tue, Apr 23, 2013 at 5:03 PM, Lee Jones <[email protected]> wrote:
> The MUSB HDRC configuration never changes between each of the ux500
> supported platforms, so there's little point passing it though platform
> data. If we set it in the driver instead, we can make good use of it
> when booting with either ATAGs or Device Tree.
>
> Cc: Felipe Balbi <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Tue, Apr 23, 2013 at 5:03 PM, Lee Jones <[email protected]> wrote:
> The dma_mask will always be the same as the coherent_dma_mask, so let's
> cut down on the platform_data burden and set it as such in the driver.
> This also saves us from supporting it separately when we come to enable
> this driver for Device Tree.
>
> Cc: Felipe Balbi <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
And makes it way more readable too!
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Wed, Apr 24, 2013 at 5:04 PM, Lee Jones <[email protected]> wrote:
> In its current state, the ux500-musb driver uses platform data pointers
> blindly with no prior checking. If no platform data pointer is passed
> this will Oops the kernel. In this patch we ensure platform data and
> board data are present prior to using them.
>
> Cc: Felipe Balbi <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Wed, Apr 24, 2013 at 5:05 PM, Lee Jones <[email protected]> wrote:
> If we can ever get to a state where we can solely search for DMA channels
> by name, this will almost completely alleviate the requirement to pass
> copious amounts of information though platform data. Here we take the
> first step towards this. The next step will be to enable Device Tree
> complete with name<->event_line mapping.
>
> Cc: Felipe Balbi <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
Looking good, would be nice if Arnd checked it tho.
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Wed, Apr 24, 2013 at 9:43 AM, Lee Jones <[email protected]> wrote:
> usb: musb: ux500: add device tree probing support
>
> This patch will allow ux500-musb to be probed and configured solely from
> configuration found in Device Tree.
>
> Cc: Felipe Balbi <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
This patch needs to be CC: devicetree-discuss.
But looks good to me!
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Tue, Apr 23, 2013 at 5:03 PM, Lee Jones <[email protected]> wrote:
> The recently DT:ed MUSB driver will require clock-name by device-name
> look-up capability, until common clk has is properly supported by the
> ux500 platform.
>
> Signed-off-by: Lee Jones <[email protected]>
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Wed, Apr 24, 2013 at 9:41 AM, Lee Jones <[email protected]> wrote:
> This patch provides all the information to successfully probe() and
> correctly configure the ux500-musb device driver for DMA.
>
> Signed-off-by: Lee Jones <[email protected]>
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Tue, Apr 23, 2013 at 5:03 PM, Lee Jones <[email protected]> wrote:
> Now the ux500-musb driver has been enabled for Device Tree, there is no
> requirement to register it from platform code.
>
> Signed-off-by: Lee Jones <[email protected]>
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Tue, Apr 23, 2013 at 5:03 PM, Lee Jones <[email protected]> wrote:
> As promised, now all devices which resided in u8500_of_init_devices()
> have been enabled for Device Tree, we can completely remove it.
>
> Signed-off-by: Lee Jones <[email protected]>
Beautiful.
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
Hi Lee,
On Tue, Apr 23, 2013 at 04:03:04PM +0100, Lee Jones wrote:
> In this patch-set we firstly ease the burden on data being passed from
> platform data, then start to build up support to finally enable Device
> Tree.
The whole series looks good to me, nice job!!
Acked-by: Fabio Baltieri <[email protected]>
I was not able to apply the patches though, as I can imagine that there
are some dependencies with your other dma40 series. If you can provide a
tree I would be happy to run some test for both host and device mode.
I still have a bunch of USB cables and adapters tangled together on my
desk ready for that.
Fabio
--
Fabio Baltieri
On Fri, 26 Apr 2013, Fabio Baltieri wrote:
> Hi Lee,
>
> On Tue, Apr 23, 2013 at 04:03:04PM +0100, Lee Jones wrote:
> > In this patch-set we firstly ease the burden on data being passed from
> > platform data, then start to build up support to finally enable Device
> > Tree.
>
> The whole series looks good to me, nice job!!
>
> Acked-by: Fabio Baltieri <[email protected]>
Thanks for reviewing.
> I was not able to apply the patches though, as I can imagine that there
> are some dependencies with your other dma40 series. If you can provide a
> tree I would be happy to run some test for both host and device mode.
> I still have a bunch of USB cables and adapters tangled together on my
> desk ready for that.
The most recent branch on my public tree contains the latest code.
Memcpy will probably fail though, as I haven't fixed up Linus'
comments pertaining them that yet.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Tue, 23 Apr 2013, Lee Jones wrote:
> Now the ux500-musb driver has been enabled for Device Tree, there is no
> requirement to register it from platform code.
>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> arch/arm/mach-ux500/cpu-db8500.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
> index 5a6b9ae..36ef6ed 100644
> --- a/arch/arm/mach-ux500/cpu-db8500.c
> +++ b/arch/arm/mach-ux500/cpu-db8500.c
> @@ -227,8 +227,6 @@ static struct device * __init u8500_of_init_devices(void)
> {
> struct device *parent = db8500_soc_device_init();
>
> - db8500_add_usb(parent, usb_db8500_dma_cfg, usb_db8500_dma_cfg);
> -
> return parent;
> }
Linus, poke.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Tue, 23 Apr 2013, Lee Jones wrote:
> As promised, now all devices which resided in u8500_of_init_devices()
> have been enabled for Device Tree, we can completely remove it.
>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> arch/arm/mach-ux500/cpu-db8500.c | 14 +-------------
> 1 file changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
> index 36ef6ed..a2c83c2 100644
> --- a/arch/arm/mach-ux500/cpu-db8500.c
> +++ b/arch/arm/mach-ux500/cpu-db8500.c
> @@ -221,15 +221,6 @@ struct device * __init u8500_init_devices(struct ab8500_platform_data *ab8500)
> }
>
> #ifdef CONFIG_MACH_UX500_DT
> -
> -/* TODO: Once all pieces are DT:ed, remove completely. */
> -static struct device * __init u8500_of_init_devices(void)
> -{
> - struct device *parent = db8500_soc_device_init();
> -
> - return parent;
> -}
> -
> static struct of_dev_auxdata u8500_auxdata_lookup[] __initdata = {
> /* Requires call-back bindings. */
> OF_DEV_AUXDATA("arm,cortex-a9-pmu", 0, "arm-pmu", &db8500_pmu_platdata),
> @@ -288,7 +279,7 @@ static const struct of_device_id u8500_local_bus_nodes[] = {
>
> static void __init u8500_init_machine(void)
> {
> - struct device *parent = NULL;
> + struct device *parent = db8500_soc_device_init();
>
> /* Pinmaps must be in place before devices register */
> if (of_machine_is_compatible("st-ericsson,mop500"))
> @@ -301,9 +292,6 @@ static void __init u8500_init_machine(void)
> else if (of_machine_is_compatible("st-ericsson,ccu9540")) {}
> /* TODO: Add pinmaps for ccu9540 board. */
>
> - /* TODO: Export SoC, USB, cpu-freq and DMA40 */
> - parent = u8500_of_init_devices();
> -
> /* automatically probe child nodes of db8500 device */
> of_platform_populate(NULL, u8500_local_bus_nodes, u8500_auxdata_lookup, parent);
> }
Linus, poke.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog