[PATCH 0/4] can: flexcan: upgrade the flexcan.c to support i.MX6

[PATCH 0/4] can: flexcan: upgrade the flexcan.c to support i.MX6

[Hui Wang]

i.MX6 is freescale arm chip, it has two flexcan controllers, when i add this driver to the i.MX6 and run it on the i.MX6 sabre lite board, this driver has several problems:
1. the value in the device tree is stored in big endian format, while arm works in little endian mode by default, The [1/4] patch fix this problem.
2. the i.MX6 has a higher can controller and has more registers, and some registers need to be set before the hardware can work well, The [2/4] patch fix this problem.
3. i.MX6 has two clocks to drive flexcan module. The [3/4] patch fix this problem.
4. i.MX6 has an external PHY to be operated. The [4/4] patch fix this problem.

Hui Wang (4):
      can: flexcan: use be32_to_cpup to handle the value of dt entry
      can: flexcan: add hardware controller version support
      can: flexcan: add ipg and ser clocks support
      can: flexcan: add transceiver switch support when use device tree

 .../devicetree/bindings/net/can/fsl-flexcan.txt    |    7 +
 drivers/net/can/flexcan.c                          |  119 ++++++++++++++++----
 2 files changed, 106 insertions(+), 20 deletions(-)

Thanks,
Hui.

[PATCH 1/4] can: flexcan: use be32_to_cpup to handle the value of dt entry

[Hui Wang]

The freescale arm i.MX series platform can support this driver, and
usually the arm cpu works in the little endian mode by default, while
device tree entry value is stored in big endian format, we should use
be32_to_cpup() to handle them, after modification, it can work well
both on the le cpu and be cpu.

Cc: linux-can@vger.kernel.org
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Hui Wang <jason77.wang@gmail.com>
---
 drivers/net/can/flexcan.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 38c0690..81d4741 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -939,12 +939,12 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 		return PTR_ERR(pinctrl);
 
 	if (pdev->dev.of_node) {
-		const u32 *clock_freq_p;
+		const __be32 *clock_freq_p;
 
 		clock_freq_p = of_get_property(pdev->dev.of_node,
 						"clock-frequency", NULL);
 		if (clock_freq_p)
-			clock_freq = *clock_freq_p;
+			clock_freq = be32_to_cpup(clock_freq_p);
 	}
 
 	if (!clock_freq) {
-- 
1.7.6

[PATCH 2/4] can: flexcan: add hardware controller version support

[Hui Wang]

At least in the i.MX series, the flexcan contrller divides into ver_3
and ver_10, current driver is for ver_3 controller.

i.MX6 has ver_10 controller, it has more reigsters than ver_3 has.
The rxfgmask (Rx FIFO Global Mask) register is one of the new added.
Its reset value is 0xffffffff, this means ID Filter Table must be
checked when receive a packet, but the driver is designed to accept
everything during the chip start, we need to clear this register to
follow this design.

Add a hw_ver entry in the device tree, this can let us distinguish
which version the controller is, if we don't set value to this entry,
the hw_ver is 3 by default, this is backward compatible for existing
platforms like powerpc and imx35.

Cc: linux-can@vger.kernel.org
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Hui Wang <jason77.wang@gmail.com>
---
 .../devicetree/bindings/net/can/fsl-flexcan.txt    |    3 +++
 drivers/net/can/flexcan.c                          |   20 +++++++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index f31b686..19952cd 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -12,6 +12,8 @@ Required properties:
 - reg : Offset and length of the register set for this device
 - interrupts : Interrupt tuple for this device
 - clock-frequency : The oscillator frequency driving the flexcan device
+- hw-version : The controller version, for imx25, imx35 and imx53, the version
+               is 3, for imx6, the version is 10
 
 Example:
 
@@ -21,4 +23,5 @@ Example:
 		interrupts = <48 0x2>;
 		interrupt-parent = <&mpic>;
 		clock-frequency = <200000000>; // filled in by bootloader
+		hw-version = <10>;
 	};
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 81d4741..9d024a4 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -165,7 +165,13 @@ struct flexcan_regs {
 	u32 imask1;		/* 0x28 */
 	u32 iflag2;		/* 0x2c */
 	u32 iflag1;		/* 0x30 */
-	u32 _reserved2[19];
+	u32 crl2;		/* 0x34 */
+	u32 esr2;		/* 0x38 */
+	u32 _reserved2[2];
+	u32 crcr;		/* 0x44 */
+	u32 rxfgmask;		/* 0x48 */
+	u32 rxfir;		/* 0x4c */
+	u32 _reserved3[12];
 	struct flexcan_mb cantxfg[64];
 };
 
@@ -177,6 +183,7 @@ struct flexcan_priv {
 	void __iomem *base;
 	u32 reg_esr;
 	u32 reg_ctrl_default;
+	u32 hw_ver;
 
 	struct clk *clk;
 	struct flexcan_platform_data *pdata;
@@ -750,6 +757,9 @@ static int flexcan_chip_start(struct net_device *dev)
 	flexcan_write(0x0, &regs->rx14mask);
 	flexcan_write(0x0, &regs->rx15mask);
 
+	if (priv->hw_ver >= 10)
+		flexcan_write(0x0, &regs->rxfgmask);
+
 	flexcan_transceiver_switch(priv, 1);
 
 	/* synchronize with the can bus */
@@ -933,6 +943,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	resource_size_t mem_size;
 	int err, irq;
 	u32 clock_freq = 0;
+	u32 hw_ver = 3;
 
 	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
 	if (IS_ERR(pinctrl))
@@ -940,11 +951,17 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 
 	if (pdev->dev.of_node) {
 		const __be32 *clock_freq_p;
+		const __be32 *hw_ver_p;
 
 		clock_freq_p = of_get_property(pdev->dev.of_node,
 						"clock-frequency", NULL);
 		if (clock_freq_p)
 			clock_freq = be32_to_cpup(clock_freq_p);
+
+		hw_ver_p = of_get_property(pdev->dev.of_node,
+						"hw-version", NULL);
+		if (hw_ver_p)
+			hw_ver = be32_to_cpup(hw_ver_p);
 	}
 
 	if (!clock_freq) {
@@ -997,6 +1014,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	priv->base = base;
 	priv->dev = dev;
 	priv->clk = clk;
+	priv->hw_ver = hw_ver;
 	priv->pdata = pdev->dev.platform_data;
 
 	netif_napi_add(dev, &priv->napi, flexcan_poll, FLEXCAN_NAPI_WEIGHT);
-- 
1.7.6

[PATCH 3/4] can: flexcan: add ipg and ser clocks support

[Hui Wang]

Some i.MX chip (like i.MX6) has two clocks to drive flexcan module,
one is ipg clock, another is serial clock, if we want the flexcan
to work, we need to enable both two clocks, but current driver only
support one clock.

My modification doesn't break existing platforms. For those without
clocks (like powerpc platforms), two clk pointers are set to NULL as
before, for those only have one clock (like i.MX25 and i.MX35), the
second clk pointer is set to NULL.

Cc: linux-can@vger.kernel.org
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Hui Wang <jason77.wang@gmail.com>
---
 drivers/net/can/flexcan.c |   49 +++++++++++++++++++++++++++++---------------
 1 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 9d024a4..a23c11e 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -185,7 +185,8 @@ struct flexcan_priv {
 	u32 reg_ctrl_default;
 	u32 hw_ver;
 
-	struct clk *clk;
+	struct clk *clk_ipg;
+	struct clk *clk_per;
 	struct flexcan_platform_data *pdata;
 };
 
@@ -814,7 +815,8 @@ static int flexcan_open(struct net_device *dev)
 	struct flexcan_priv *priv = netdev_priv(dev);
 	int err;
 
-	clk_prepare_enable(priv->clk);
+	clk_prepare_enable(priv->clk_ipg);
+	clk_prepare_enable(priv->clk_per);
 
 	err = open_candev(dev);
 	if (err)
@@ -836,7 +838,8 @@ static int flexcan_open(struct net_device *dev)
  out_close:
 	close_candev(dev);
  out:
-	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->clk_ipg);
+	clk_disable_unprepare(priv->clk_per);
 
 	return err;
 }
@@ -850,7 +853,8 @@ static int flexcan_close(struct net_device *dev)
 	flexcan_chip_stop(dev);
 
 	free_irq(dev->irq, dev);
-	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->clk_ipg);
+	clk_disable_unprepare(priv->clk_per);
 
 	close_candev(dev);
 
@@ -889,7 +893,8 @@ static int __devinit register_flexcandev(struct net_device *dev)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg, err;
 
-	clk_prepare_enable(priv->clk);
+	clk_prepare_enable(priv->clk_ipg);
+	clk_prepare_enable(priv->clk_per);
 
 	/* select "bus clock", chip must be disabled */
 	flexcan_chip_disable(priv);
@@ -922,7 +927,8 @@ static int __devinit register_flexcandev(struct net_device *dev)
  out:
 	/* disable core and turn off clocks */
 	flexcan_chip_disable(priv);
-	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->clk_ipg);
+	clk_disable_unprepare(priv->clk_per);
 
 	return err;
 }
@@ -937,7 +943,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	struct net_device *dev;
 	struct flexcan_priv *priv;
 	struct resource *mem;
-	struct clk *clk = NULL;
+	struct clk *clk_per = NULL;
+	struct clk *clk_ipg = NULL;
 	struct pinctrl *pinctrl;
 	void __iomem *base;
 	resource_size_t mem_size;
@@ -965,15 +972,18 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	}
 
 	if (!clock_freq) {
-		clk = clk_get(&pdev->dev, NULL);
-		if (IS_ERR(clk)) {
+		clk_ipg = clk_get(&pdev->dev, "ipg");
+		if (IS_ERR(clk_ipg)) {
 			dev_err(&pdev->dev, "no clock defined\n");
-			err = PTR_ERR(clk);
+			err = PTR_ERR(clk_ipg);
 			goto failed_clock;
 		}
-		clock_freq = clk_get_rate(clk);
-	}
+		clock_freq = clk_get_rate(clk_ipg);
 
+		clk_per = clk_get(&pdev->dev, "per");
+		if (IS_ERR(clk_per) || clk_per == clk_ipg)
+			clk_per = NULL;
+	}
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq = platform_get_irq(pdev, 0);
 	if (!mem || irq <= 0) {
@@ -1013,7 +1023,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 		CAN_CTRLMODE_BERR_REPORTING;
 	priv->base = base;
 	priv->dev = dev;
-	priv->clk = clk;
+	priv->clk_ipg = clk_ipg;
+	priv->clk_per = clk_per;
 	priv->hw_ver = hw_ver;
 	priv->pdata = pdev->dev.platform_data;
 
@@ -1040,8 +1051,10 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
  failed_map:
 	release_mem_region(mem->start, mem_size);
  failed_get:
-	if (clk)
-		clk_put(clk);
+	if (clk_per)
+		clk_put(clk_per);
+	if (clk_ipg)
+		clk_put(clk_ipg);
  failed_clock:
 	return err;
 }
@@ -1059,8 +1072,10 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(mem->start, resource_size(mem));
 
-	if (priv->clk)
-		clk_put(priv->clk);
+	if (priv->clk_per)
+		clk_put(priv->clk_per);
+	if (priv->clk_ipg)
+		clk_put(priv->clk_ipg);
 
 	free_candev(dev);
 
-- 
1.7.6

[PATCH 4/4] can: flexcan: add transceiver switch support when use device tree

[Hui Wang]

Some platforms (like i.MX6) has an external PHY, the PHY is operated
by some gpios. If the system registers a platform_data, we can
set a callback function pointer to pdata->transceiver_switch to
implement PHY switch. If we use device tree, we couldn't pass
platform_data to the driver, so i move the switch function to the
driver and add device tree entries to let user set which gpios
are used to operate PHY.

This design doesn't break existing platforms, if a platform doesn't
need PHY switch, it doesn't need to set dt entries just as before,
if a platform has platform_data, the pdata->transceiver_switch has
higher priority.

Cc: linux-can@vger.kernel.org
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Hui Wang <jason77.wang@gmail.com>
---
 .../devicetree/bindings/net/can/fsl-flexcan.txt    |    4 ++
 drivers/net/can/flexcan.c                          |   46 ++++++++++++++++++++
 2 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index 19952cd..a32bfdb 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -14,6 +14,8 @@ Required properties:
 - clock-frequency : The oscillator frequency driving the flexcan device
 - hw-version : The controller version, for imx25, imx35 and imx53, the version
                is 3, for imx6, the version is 10
+- phy-en-gpio : Specify the gpio to be used as phy enable
+- phy-stby-gpio : Specify the gpio to be used as phy standby
 
 Example:
 
@@ -24,4 +26,6 @@ Example:
 		interrupt-parent = <&mpic>;
 		clock-frequency = <200000000>; // filled in by bootloader
 		hw-version = <10>;
+		phy-en-gpio = <&gpio1 4 0>; /* GPIO1_4 */
+		phy-stby-gpio = <&gpio1 2 0>; /* GPIO1_2 */
 	};
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index a23c11e..0318584 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -36,6 +36,7 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/of_gpio.h>
 
 #define DRV_NAME			"flexcan"
 
@@ -184,6 +185,8 @@ struct flexcan_priv {
 	u32 reg_esr;
 	u32 reg_ctrl_default;
 	u32 hw_ver;
+	int phy_en_gpio;
+	int phy_stby_gpio;
 
 	struct clk *clk_ipg;
 	struct clk *clk_per;
@@ -234,6 +237,15 @@ static void flexcan_transceiver_switch(const struct flexcan_priv *priv, int on)
 {
 	if (priv->pdata && priv->pdata->transceiver_switch)
 		priv->pdata->transceiver_switch(on);
+	else if (priv->phy_en_gpio >= 0 && priv->phy_stby_gpio >= 0) {
+		if (on) {
+			gpio_set_value(priv->phy_en_gpio, 1);
+			gpio_set_value(priv->phy_stby_gpio, 1);
+		} else {
+			gpio_set_value(priv->phy_en_gpio, 0);
+			gpio_set_value(priv->phy_stby_gpio, 0);
+		}
+	}
 }
 
 static inline int flexcan_has_and_handle_berr(const struct flexcan_priv *priv,
@@ -951,6 +963,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	int err, irq;
 	u32 clock_freq = 0;
 	u32 hw_ver = 3;
+	int en_gpio = -EINVAL;
+	int stby_gpio = -EINVAL;
 
 	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
 	if (IS_ERR(pinctrl))
@@ -969,6 +983,26 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 						"hw-version", NULL);
 		if (hw_ver_p)
 			hw_ver = be32_to_cpup(hw_ver_p);
+
+		en_gpio = of_get_named_gpio(pdev->dev.of_node, "phy-en-gpio", 0);
+		if (en_gpio >= 0) {
+			int ret;
+			ret = gpio_request(en_gpio, pdev->name);
+			if (ret)
+				en_gpio = -EINVAL;
+			else
+				gpio_direction_output(en_gpio, 0);
+		}
+
+		stby_gpio = of_get_named_gpio(pdev->dev.of_node, "phy-stby-gpio", 0);
+		if (stby_gpio >= 0) {
+			int ret;
+			ret = gpio_request(stby_gpio, pdev->name);
+			if (ret)
+				stby_gpio = -EINVAL;
+			else
+				gpio_direction_output(stby_gpio, 0);
+		}
 	}
 
 	if (!clock_freq) {
@@ -1026,6 +1060,9 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	priv->clk_ipg = clk_ipg;
 	priv->clk_per = clk_per;
 	priv->hw_ver = hw_ver;
+	priv->phy_en_gpio = en_gpio;
+	priv->phy_stby_gpio = stby_gpio;
+
 	priv->pdata = pdev->dev.platform_data;
 
 	netif_napi_add(dev, &priv->napi, flexcan_poll, FLEXCAN_NAPI_WEIGHT);
@@ -1056,6 +1093,10 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	if (clk_ipg)
 		clk_put(clk_ipg);
  failed_clock:
+	if (en_gpio >= 0)
+		gpio_free(en_gpio);
+	if (stby_gpio >= 0)
+		gpio_free(stby_gpio);
 	return err;
 }
 
@@ -1077,6 +1118,11 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
 	if (priv->clk_ipg)
 		clk_put(priv->clk_ipg);
 
+	if (priv->phy_en_gpio >= 0)
+		gpio_free(priv->phy_en_gpio);
+	if (priv->phy_stby_gpio >= 0)
+		gpio_free(priv->phy_stby_gpio);
+
 	free_candev(dev);
 
 	return 0;
-- 
1.7.6

Re: [PATCH 4/4] can: flexcan: add transceiver switch support when use device tree

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]

On 06/27/2012 10:19 AM, Hui Wang wrote:
> Some platforms (like i.MX6) has an external PHY, the PHY is operated
> by some gpios. If the system registers a platform_data, we can
> set a callback function pointer to pdata->transceiver_switch to
> implement PHY switch. If we use device tree, we couldn't pass
> platform_data to the driver, so i move the switch function to the
> driver and add device tree entries to let user set which gpios
> are used to operate PHY.
> 
> This design doesn't break existing platforms, if a platform doesn't
> need PHY switch, it doesn't need to set dt entries just as before,
> if a platform has platform_data, the pdata->transceiver_switch has
> higher priority.
> 
> Cc: linux-can@vger.kernel.org
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Signed-off-by: Hui Wang <jason77.wang@gmail.com>

please coordinate whith Shawn Guo, he has posted a similar patch some
days ago. http://www.spinics.net/lists/netdev/msg202442.html

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 2/4] can: flexcan: add hardware controller version support

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

On 06/27/2012 10:19 AM, Hui Wang wrote:
> At least in the i.MX series, the flexcan contrller divides into ver_3
> and ver_10, current driver is for ver_3 controller.
> 
> i.MX6 has ver_10 controller, it has more reigsters than ver_3 has.
> The rxfgmask (Rx FIFO Global Mask) register is one of the new added.
> Its reset value is 0xffffffff, this means ID Filter Table must be
> checked when receive a packet, but the driver is designed to accept
> everything during the chip start, we need to clear this register to
> follow this design.
> 
> Add a hw_ver entry in the device tree, this can let us distinguish
> which version the controller is, if we don't set value to this entry,
> the hw_ver is 3 by default, this is backward compatible for existing
> platforms like powerpc and imx35.

Is it possible to read this value from the hardware?
Another possibility would be to introduce a new compatible device in the
device tree.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 1/4] can: flexcan: use be32_to_cpup to handle the value of dt entry

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1880 bytes --]

On 06/27/2012 10:19 AM, Hui Wang wrote:
> The freescale arm i.MX series platform can support this driver, and
> usually the arm cpu works in the little endian mode by default, while
> device tree entry value is stored in big endian format, we should use
> be32_to_cpup() to handle them, after modification, it can work well
> both on the le cpu and be cpu.

good catch. We didn't notice since this clock has only been used on
powerpc so far. However you should not need to set this property, as ARM
has proper clock tree support and the clock rate is taken from the clock
tree not the device tree.

Applied to can-master.

Marc

> Cc: linux-can@vger.kernel.org
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Signed-off-by: Hui Wang <jason77.wang@gmail.com>
> ---
>  drivers/net/can/flexcan.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 38c0690..81d4741 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -939,12 +939,12 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>  		return PTR_ERR(pinctrl);
>  
>  	if (pdev->dev.of_node) {
> -		const u32 *clock_freq_p;
> +		const __be32 *clock_freq_p;
>  
>  		clock_freq_p = of_get_property(pdev->dev.of_node,
>  						"clock-frequency", NULL);
>  		if (clock_freq_p)
> -			clock_freq = *clock_freq_p;
> +			clock_freq = be32_to_cpup(clock_freq_p);
>  	}
>  
>  	if (!clock_freq) {


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 3/4] can: flexcan: add ipg and ser clocks support

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 883 bytes --]

On 06/27/2012 10:19 AM, Hui Wang wrote:
> Some i.MX chip (like i.MX6) has two clocks to drive flexcan module,
> one is ipg clock, another is serial clock, if we want the flexcan
> to work, we need to enable both two clocks, but current driver only
> support one clock.
> 
> My modification doesn't break existing platforms. For those without
> clocks (like powerpc platforms), two clk pointers are set to NULL as
> before, for those only have one clock (like i.MX25 and i.MX35), the
> second clk pointer is set to NULL.

Looks good at the first sight. Will test on hardware later.

Tnx, Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 2/4] can: flexcan: add hardware controller version support

[Wolfgang Grandegger]

On 06/27/2012 10:27 AM, Marc Kleine-Budde wrote:
> On 06/27/2012 10:19 AM, Hui Wang wrote:
>> At least in the i.MX series, the flexcan contrller divides into ver_3
>> and ver_10, current driver is for ver_3 controller.
>>
>> i.MX6 has ver_10 controller, it has more reigsters than ver_3 has.
>> The rxfgmask (Rx FIFO Global Mask) register is one of the new added.
>> Its reset value is 0xffffffff, this means ID Filter Table must be
>> checked when receive a packet, but the driver is designed to accept
>> everything during the chip start, we need to clear this register to
>> follow this design.
>>
>> Add a hw_ver entry in the device tree, this can let us distinguish
>> which version the controller is, if we don't set value to this entry,
>> the hw_ver is 3 by default, this is backward compatible for existing
>> platforms like powerpc and imx35.
> 
> Is it possible to read this value from the hardware?
> Another possibility would be to introduce a new compatible device in the
> device tree.

I vote for the latter. IIRC, in the past we already had some discussion
on how to handle version dependent Flexcan hardware, e.g. by using
flexcan-vX.X or being expicit using fsl,p1010-flexcan. Search for "Add
support for powerpc" in the netdev mailing list. I added the
devicetree-discuss ml for that reason.

Wolfgang.

Re: [PATCH 1/4] can: flexcan: use be32_to_cpup to handle the value of dt entry

[Wolfgang Grandegger]

On 06/27/2012 10:19 AM, Hui Wang wrote:
> The freescale arm i.MX series platform can support this driver, and
> usually the arm cpu works in the little endian mode by default, while
> device tree entry value is stored in big endian format, we should use
> be32_to_cpup() to handle them, after modification, it can work well
> both on the le cpu and be cpu.

Oops, that reminds me that other Linux-CAN (of-)drivers do require a
similar fix to work on little endian platforms. Not your business, of
course.

Wolfgang.

Re: [PATCH 2/4] can: flexcan: add hardware controller version support

[Wolfgang Grandegger]

Hi Marc,

On 06/27/2012 10:56 AM, Wolfgang Grandegger wrote:
> On 06/27/2012 10:27 AM, Marc Kleine-Budde wrote:
>> On 06/27/2012 10:19 AM, Hui Wang wrote:
>>> At least in the i.MX series, the flexcan contrller divides into ver_3
>>> and ver_10, current driver is for ver_3 controller.
>>>
>>> i.MX6 has ver_10 controller, it has more reigsters than ver_3 has.
>>> The rxfgmask (Rx FIFO Global Mask) register is one of the new added.
>>> Its reset value is 0xffffffff, this means ID Filter Table must be
>>> checked when receive a packet, but the driver is designed to accept
>>> everything during the chip start, we need to clear this register to
>>> follow this design.
>>>
>>> Add a hw_ver entry in the device tree, this can let us distinguish
>>> which version the controller is, if we don't set value to this entry,
>>> the hw_ver is 3 by default, this is backward compatible for existing
>>> platforms like powerpc and imx35.
>>
>> Is it possible to read this value from the hardware?
>> Another possibility would be to introduce a new compatible device in the
>> device tree.
> 
> I vote for the latter. IIRC, in the past we already had some discussion
> on how to handle version dependent Flexcan hardware, e.g. by using
> flexcan-vX.X or being expicit using fsl,p1010-flexcan. Search for "Add
> support for powerpc" in the netdev mailing list. I added the
> devicetree-discuss ml for that reason.

I looked up the threads and found:

  http://marc.info/?w=4&r=1&s=Fix+up+fsl-flexcan+device+tree+bi&q=t

In the Flexcan driver we currently only have:

  static struct of_device_id flexcan_of_match[] = {
        {
                .compatible = "fsl,p1010-flexcan",
        },
        {},
  };

What compatible string do they actually use for the i.MX6Q board? Shawn
or Hui? We need to fix that. From the discussion mentioned above I think
"fsl,flexcan-v10" would be acceptable. Unfortunately, we do not known
the internal version of the Flexcan controllers used in the various
PowerPC and ARM SOCs. We already realized some differences with
interrupts and bus error handling between i.MX28 and i.MX35. Would be
nice if someone (from Freescale?) could finally clarify that.

Wolfgang.

Re: [PATCH 2/4] can: flexcan: add hardware controller version support

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 2815 bytes --]

On 06/27/2012 11:43 AM, Wolfgang Grandegger wrote:
> Hi Marc,
> 
> On 06/27/2012 10:56 AM, Wolfgang Grandegger wrote:
>> On 06/27/2012 10:27 AM, Marc Kleine-Budde wrote:
>>> On 06/27/2012 10:19 AM, Hui Wang wrote:
>>>> At least in the i.MX series, the flexcan contrller divides into ver_3
>>>> and ver_10, current driver is for ver_3 controller.
>>>>
>>>> i.MX6 has ver_10 controller, it has more reigsters than ver_3 has.
>>>> The rxfgmask (Rx FIFO Global Mask) register is one of the new added.
>>>> Its reset value is 0xffffffff, this means ID Filter Table must be
>>>> checked when receive a packet, but the driver is designed to accept
>>>> everything during the chip start, we need to clear this register to
>>>> follow this design.
>>>>
>>>> Add a hw_ver entry in the device tree, this can let us distinguish
>>>> which version the controller is, if we don't set value to this entry,
>>>> the hw_ver is 3 by default, this is backward compatible for existing
>>>> platforms like powerpc and imx35.
>>>
>>> Is it possible to read this value from the hardware?
>>> Another possibility would be to introduce a new compatible device in the
>>> device tree.
>>
>> I vote for the latter. IIRC, in the past we already had some discussion
>> on how to handle version dependent Flexcan hardware, e.g. by using
>> flexcan-vX.X or being expicit using fsl,p1010-flexcan. Search for "Add
>> support for powerpc" in the netdev mailing list. I added the
>> devicetree-discuss ml for that reason.
> 
> I looked up the threads and found:
> 
>   http://marc.info/?w=4&r=1&s=Fix+up+fsl-flexcan+device+tree+bi&q=t
> 
> In the Flexcan driver we currently only have:
> 
>   static struct of_device_id flexcan_of_match[] = {
>         {
>                 .compatible = "fsl,p1010-flexcan",
>         },
>         {},
>   };
> 
> What compatible string do they actually use for the i.MX6Q board? Shawn
> or Hui? We need to fix that. From the discussion mentioned above I think
> "fsl,flexcan-v10" would be acceptable. Unfortunately, we do not known

As far as I understand the DT, the name should be something like

"fsl,${OLDEST_SOCK_THAT_HAS_THIS_VERSION_OF_FLEXCAN}-flexcan".

> the internal version of the Flexcan controllers used in the various
> PowerPC and ARM SOCs. We already realized some differences with
> interrupts and bus error handling between i.MX28 and i.MX35. Would be
> nice if someone (from Freescale?) could finally clarify that.

That information would be interesting.

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 4/4] can: flexcan: add transceiver switch support when use device tree

[Hui Wang]

Marc Kleine-Budde wrote:
> On 06/27/2012 10:19 AM, Hui Wang wrote:
>   
>> Some platforms (like i.MX6) has an external PHY, the PHY is operated
>> by some gpios. If the system registers a platform_data, we can
>> set a callback function pointer to pdata->transceiver_switch to
>> implement PHY switch. If we use device tree, we couldn't pass
>> platform_data to the driver, so i move the switch function to the
>> driver and add device tree entries to let user set which gpios
>> are used to operate PHY.
>>
>> This design doesn't break existing platforms, if a platform doesn't
>> need PHY switch, it doesn't need to set dt entries just as before,
>> if a platform has platform_data, the pdata->transceiver_switch has
>> higher priority.
>>
>> Cc: linux-can@vger.kernel.org
>> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Shawn Guo <shawn.guo@linaro.org>
>> Signed-off-by: Hui Wang <jason77.wang@gmail.com>
>>     
>
> please coordinate whith Shawn Guo, he has posted a similar patch some
> days ago. http://www.spinics.net/lists/netdev/msg202442.html
>
> Marc
>
>   
After read and compared with his patch:
1. Shawn use gpio_is_valid(gpio) instead of (gpio >= 0), it is good.
2. Shawn add a flag to record active level, it is good.
3. Shawn only add 1 gpio, this is not enough for imx6 sabre lite board.
4. Shawn forget to call gpio_release()


Shawn, what about your opinion?

Regards,
Hui.

Re: [PATCH 4/4] can: flexcan: add transceiver switch support when use device tree

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1955 bytes --]

On 06/27/2012 11:55 AM, Hui Wang wrote:
> Marc Kleine-Budde wrote:
>> On 06/27/2012 10:19 AM, Hui Wang wrote:
>>  
>>> Some platforms (like i.MX6) has an external PHY, the PHY is operated
>>> by some gpios. If the system registers a platform_data, we can
>>> set a callback function pointer to pdata->transceiver_switch to
>>> implement PHY switch. If we use device tree, we couldn't pass
>>> platform_data to the driver, so i move the switch function to the
>>> driver and add device tree entries to let user set which gpios
>>> are used to operate PHY.
>>>
>>> This design doesn't break existing platforms, if a platform doesn't
>>> need PHY switch, it doesn't need to set dt entries just as before,
>>> if a platform has platform_data, the pdata->transceiver_switch has
>>> higher priority.
>>>
>>> Cc: linux-can@vger.kernel.org
>>> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
>>> Cc: David S. Miller <davem@davemloft.net>
>>> Cc: Shawn Guo <shawn.guo@linaro.org>
>>> Signed-off-by: Hui Wang <jason77.wang@gmail.com>
>>>     
>>
>> please coordinate whith Shawn Guo, he has posted a similar patch some
>> days ago. http://www.spinics.net/lists/netdev/msg202442.html
>>
>> Marc
>>
>>   
> After read and compared with his patch:
> 1. Shawn use gpio_is_valid(gpio) instead of (gpio >= 0), it is good.

yes

> 2. Shawn add a flag to record active level, it is good.

yes - have a look at the device tree if there is a common symbol name
for this.

> 3. Shawn only add 1 gpio, this is not enough for imx6 sabre lite board.

improve your patch to work with 1 or 2 gpios.

> 4. Shawn forget to call gpio_release()

good catch.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 2/4] can: flexcan: add hardware controller version support

[Hui Wang]

Marc Kleine-Budde wrote:
> On 06/27/2012 11:43 AM, Wolfgang Grandegger wrote:
>   
>> Hi Marc,
>>
>> On 06/27/2012 10:56 AM, Wolfgang Grandegger wrote:
>>     
>>> On 06/27/2012 10:27 AM, Marc Kleine-Budde wrote:
>>>       
>>>> On 06/27/2012 10:19 AM, Hui Wang wrote:
>>>>         
>>>>> At least in the i.MX series, the flexcan contrller divides into ver_3
>>>>> and ver_10, current driver is for ver_3 controller.
>>>>>
>>>>> i.MX6 has ver_10 controller, it has more reigsters than ver_3 has.
>>>>> The rxfgmask (Rx FIFO Global Mask) register is one of the new added.
>>>>> Its reset value is 0xffffffff, this means ID Filter Table must be
>>>>> checked when receive a packet, but the driver is designed to accept
>>>>> everything during the chip start, we need to clear this register to
>>>>> follow this design.
>>>>>
>>>>> Add a hw_ver entry in the device tree, this can let us distinguish
>>>>> which version the controller is, if we don't set value to this entry,
>>>>> the hw_ver is 3 by default, this is backward compatible for existing
>>>>> platforms like powerpc and imx35.
>>>>>           
>>>> Is it possible to read this value from the hardware?
>>>> Another possibility would be to introduce a new compatible device in the
>>>> device tree.
>>>>         
>>> I vote for the latter. IIRC, in the past we already had some discussion
>>> on how to handle version dependent Flexcan hardware, e.g. by using
>>> flexcan-vX.X or being expicit using fsl,p1010-flexcan. Search for "Add
>>> support for powerpc" in the netdev mailing list. I added the
>>> devicetree-discuss ml for that reason.
>>>       
>> I looked up the threads and found:
>>
>>   http://marc.info/?w=4&r=1&s=Fix+up+fsl-flexcan+device+tree+bi&q=t
>>
>> In the Flexcan driver we currently only have:
>>
>>   static struct of_device_id flexcan_of_match[] = {
>>         {
>>                 .compatible = "fsl,p1010-flexcan",
>>         },
>>         {},
>>   };
>>
>> What compatible string do they actually use for the i.MX6Q board? Shawn
>> or Hui? We need to fix that. From the discussion mentioned above I think
>>     
Currently i modified the can1 DT entry in the  imx6q.dtsi like this:
            flexcan@02090000 { /* CAN1 */
                reg = <0x02090000 0x4000>;
                interrupts = <0 110 0x04>;
                hw-version = <10>;
            };

and the DT entry in the imx6q-sabrelite.dts like this:
            flexcan@02090000 { /* CAN1 */
                compatible = "fsl,imx6q-flexcan", "fsl,p1010-flexcan";
                phy-en-gpio = <&gpio1 4 0>;
                phy-stby-gpio = <&gpio1 2 0>;
                pinctrl-names = "default";
                pinctrl-0 = <&pinctrl_flexcan1_1>;
            };
if we don't use hw-version entry and use flexcan-v10, do you mean we use 
strcmp or strxxx to decide controller version?

Regards,
Hui.
>> "fsl,flexcan-v10" would be acceptable. Unfortunately, we do not known
>>     
>
> As far as I understand the DT, the name should be something like
>
> "fsl,${OLDEST_SOCK_THAT_HAS_THIS_VERSION_OF_FLEXCAN}-flexcan".
>
>   
>> the internal version of the Flexcan controllers used in the various
>> PowerPC and ARM SOCs. We already realized some differences with
>> interrupts and bus error handling between i.MX28 and i.MX35. Would be
>> nice if someone (from Freescale?) could finally clarify that.
>>     
>
> That information would be interesting.
>
> Marc
>   

Re: [PATCH 2/4] can: flexcan: add hardware controller version support

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1884 bytes --]

On 06/27/2012 12:13 PM, Hui Wang wrote:
[...]

>>> What compatible string do they actually use for the i.MX6Q board? Shawn
>>> or Hui? We need to fix that. From the discussion mentioned above I think
>>>     
> Currently i modified the can1 DT entry in the  imx6q.dtsi like this:
>            flexcan@02090000 { /* CAN1 */
>                reg = <0x02090000 0x4000>;
>                interrupts = <0 110 0x04>;
>                hw-version = <10>;
                 ^^^^^^^^^^^^^^^^^^

remove

>            };
> 
> and the DT entry in the imx6q-sabrelite.dts like this:
>            flexcan@02090000 { /* CAN1 */
>                compatible = "fsl,imx6q-flexcan", "fsl,p1010-flexcan";
                                                   ^^^^^^^^^^^^^^^^^^^

If imx6q is the first sock with this core "fsl,imx6q-flexcan" is the
official name. "fsl,p1010-flexcan" will be removed. The compatible
should go into the imx6q.dtsi

>                phy-en-gpio = <&gpio1 4 0>;
>                phy-stby-gpio = <&gpio1 2 0>;
>                pinctrl-names = "default";
>                pinctrl-0 = <&pinctrl_flexcan1_1>;
>            };
> if we don't use hw-version entry and use flexcan-v10, do you mean we use
> strcmp or strxxx to decide controller version?

No, have a look at the flexcan_of_match. The struct of_device_id has a
data pointer that can point to some arbitrary data. Define a struct
flexcan_devtype_data which has, for now, a hardware revision member.
Have a look at the imx-spi[1] driver.

Marc

[1] http://lxr.free-electrons.com/source/drivers/spi/spi-imx.c
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 2/4] can: flexcan: add hardware controller version support

[Wolfgang Grandegger]

On 06/27/2012 12:13 PM, Hui Wang wrote:
> Marc Kleine-Budde wrote:
>> On 06/27/2012 11:43 AM, Wolfgang Grandegger wrote:
>>  
>>> Hi Marc,
>>>
>>> On 06/27/2012 10:56 AM, Wolfgang Grandegger wrote:
>>>    
>>>> On 06/27/2012 10:27 AM, Marc Kleine-Budde wrote:
>>>>      
>>>>> On 06/27/2012 10:19 AM, Hui Wang wrote:
>>>>>        
>>>>>> At least in the i.MX series, the flexcan contrller divides into ver_3
>>>>>> and ver_10, current driver is for ver_3 controller.
>>>>>>
>>>>>> i.MX6 has ver_10 controller, it has more reigsters than ver_3 has.
>>>>>> The rxfgmask (Rx FIFO Global Mask) register is one of the new added.
>>>>>> Its reset value is 0xffffffff, this means ID Filter Table must be
>>>>>> checked when receive a packet, but the driver is designed to accept
>>>>>> everything during the chip start, we need to clear this register to
>>>>>> follow this design.
>>>>>>
>>>>>> Add a hw_ver entry in the device tree, this can let us distinguish
>>>>>> which version the controller is, if we don't set value to this entry,
>>>>>> the hw_ver is 3 by default, this is backward compatible for existing
>>>>>> platforms like powerpc and imx35.
>>>>>>           
>>>>> Is it possible to read this value from the hardware?
>>>>> Another possibility would be to introduce a new compatible device
>>>>> in the
>>>>> device tree.
>>>>>         
>>>> I vote for the latter. IIRC, in the past we already had some discussion
>>>> on how to handle version dependent Flexcan hardware, e.g. by using
>>>> flexcan-vX.X or being expicit using fsl,p1010-flexcan. Search for "Add
>>>> support for powerpc" in the netdev mailing list. I added the
>>>> devicetree-discuss ml for that reason.
>>>>       
>>> I looked up the threads and found:
>>>
>>>   http://marc.info/?w=4&r=1&s=Fix+up+fsl-flexcan+device+tree+bi&q=t
>>>
>>> In the Flexcan driver we currently only have:
>>>
>>>   static struct of_device_id flexcan_of_match[] = {
>>>         {
>>>                 .compatible = "fsl,p1010-flexcan",
>>>         },
>>>         {},
>>>   };
>>>
>>> What compatible string do they actually use for the i.MX6Q board? Shawn
>>> or Hui? We need to fix that. From the discussion mentioned above I think
>>>     
> Currently i modified the can1 DT entry in the  imx6q.dtsi like this:
>            flexcan@02090000 { /* CAN1 */
>                reg = <0x02090000 0x4000>;
>                interrupts = <0 110 0x04>;
>                hw-version = <10>;
>            };
> 
> and the DT entry in the imx6q-sabrelite.dts like this:
>            flexcan@02090000 { /* CAN1 */
>                compatible = "fsl,imx6q-flexcan", "fsl,p1010-flexcan";
>                phy-en-gpio = <&gpio1 4 0>;
>                phy-stby-gpio = <&gpio1 2 0>;
>                pinctrl-names = "default";
>                pinctrl-0 = <&pinctrl_flexcan1_1>;
>            };
> if we don't use hw-version entry and use flexcan-v10, do you mean we use
> strcmp or strxxx to decide controller version?

Yes, that could be done wuth that syntax, but ...

> Regards,
> Hui.
>>> "fsl,flexcan-v10" would be acceptable. Unfortunately, we do not known
>>>     
>>
>> As far as I understand the DT, the name should be something like
>>
>> "fsl,${OLDEST_SOCK_THAT_HAS_THIS_VERSION_OF_FLEXCAN}-flexcan".

... the above string is indeed the prefered one, even if I think it's
much less transparent. You should then define the .data field of "struct
of_device_id" to set the hw version.

Wolfgang.

Re: [PATCH 4/4] can: flexcan: add transceiver switch support when use device tree

[Shawn Guo]

On 27 June 2012 17:55, Hui Wang <jason77.wang@gmail.com> wrote:
> After read and compared with his patch:
> 1. Shawn use gpio_is_valid(gpio) instead of (gpio >= 0), it is good.
> 2. Shawn add a flag to record active level, it is good.
> 3. Shawn only add 1 gpio, this is not enough for imx6 sabre lite board.
> 4. Shawn forget to call gpio_release()

I'm about to resend my series to have linux-can copied and use
devm_gpio_request_one instead.  Do you want me to add STBY gpio
support into my patch?

Regards,
Shawn

Re: [PATCH 4/4] can: flexcan: add transceiver switch support when use device tree

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 877 bytes --]

On 06/27/2012 01:22 PM, Shawn Guo wrote:
> On 27 June 2012 17:55, Hui Wang <jason77.wang@gmail.com> wrote:
>> After read and compared with his patch:
>> 1. Shawn use gpio_is_valid(gpio) instead of (gpio >= 0), it is good.
>> 2. Shawn add a flag to record active level, it is good.
>> 3. Shawn only add 1 gpio, this is not enough for imx6 sabre lite board.
>> 4. Shawn forget to call gpio_release()
> 
> I'm about to resend my series to have linux-can copied and use
> devm_gpio_request_one instead.  Do you want me to add STBY gpio
> support into my patch?

devm is a good idea.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 4/4] can: flexcan: add transceiver switch support when use device tree

[Hui Wang]

Shawn Guo wrote:
> On 27 June 2012 17:55, Hui Wang <jason77.wang@gmail.com> wrote:
>   
>> After read and compared with his patch:
>> 1. Shawn use gpio_is_valid(gpio) instead of (gpio >= 0), it is good.
>> 2. Shawn add a flag to record active level, it is good.
>> 3. Shawn only add 1 gpio, this is not enough for imx6 sabre lite board.
>> 4. Shawn forget to call gpio_release()
>>     
>
> I'm about to resend my series to have linux-can copied and use
> devm_gpio_request_one instead.  Do you want me to add STBY gpio
> support into my patch?
>   
Yes.

Thanks,
Hui.
> Regards,
> Shawn
>
>