* [PATCH v2 0/5] Add flexcan support for LS1021A SoCs
@ 2015-05-14 11:33 ` Bhupesh Sharma
0 siblings, 0 replies; 41+ messages in thread
From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw)
To: arnd, linux-can, mkl
Cc: bhupesh.linux, bhupesh.sharma, Sakar.Arora, linux-arm-kernel
This patchset add the support for FlexCAN module present
on LS1021A SoCs.
LS1021A-Rev1 SoC has a broken RX FIFO mode, thereby mandating
the use of message buffers for frame reception.
LS1021A-Rev2 SoC has this issue fixed.
Bhupesh Sharma (3):
doc/bindings: Add 'endianess' optional-property for FlexCAN
controller
can: flexcan: Add ls1021a flexcan device entry
can: flexcan: Add support for non RX-FIFO mode
Sakar Arora (2):
arm/dts: Add nodes for flexcan devices present on LS1021A SoC
can: flexcan: Remodel FlexCAN register r/w APIs for BE instances
.../devicetree/bindings/net/can/fsl-flexcan.txt | 4 +
arch/arm/boot/dts/ls1021a-qds.dts | 8 +
arch/arm/boot/dts/ls1021a.dtsi | 18 +
drivers/net/can/flexcan.c | 400 +++++++++++++-------
4 files changed, 299 insertions(+), 131 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH v2 0/5] Add flexcan support for LS1021A SoCs @ 2015-05-14 11:33 ` Bhupesh Sharma 0 siblings, 0 replies; 41+ messages in thread From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw) To: linux-arm-kernel This patchset add the support for FlexCAN module present on LS1021A SoCs. LS1021A-Rev1 SoC has a broken RX FIFO mode, thereby mandating the use of message buffers for frame reception. LS1021A-Rev2 SoC has this issue fixed. Bhupesh Sharma (3): doc/bindings: Add 'endianess' optional-property for FlexCAN controller can: flexcan: Add ls1021a flexcan device entry can: flexcan: Add support for non RX-FIFO mode Sakar Arora (2): arm/dts: Add nodes for flexcan devices present on LS1021A SoC can: flexcan: Remodel FlexCAN register r/w APIs for BE instances .../devicetree/bindings/net/can/fsl-flexcan.txt | 4 + arch/arm/boot/dts/ls1021a-qds.dts | 8 + arch/arm/boot/dts/ls1021a.dtsi | 18 + drivers/net/can/flexcan.c | 400 +++++++++++++------- 4 files changed, 299 insertions(+), 131 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 1/5] doc/bindings: Add 'endianess' optional-property for FlexCAN controller 2015-05-14 11:33 ` Bhupesh Sharma @ 2015-05-14 11:33 ` Bhupesh Sharma -1 siblings, 0 replies; 41+ messages in thread From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw) To: arnd, linux-can, mkl Cc: bhupesh.sharma, bhupesh.linux, linux-arm-kernel, Sakar.Arora This patch adds 'endianess' as the optional-property for describing the FlexCAN controller present on various FSL platforms. Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> --- .../devicetree/bindings/net/can/fsl-flexcan.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt index 56d6cc3..0d5e668 100644 --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt @@ -18,6 +18,10 @@ Optional properties: - xceiver-supply: Regulator that powers the CAN transceiver +- little-endian: If the FlexCAN IP on this SoC is little-endian, use + this property. By default it is assumed that the IP + is big-endian + Example: can@1c000 { -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 1/5] doc/bindings: Add 'endianess' optional-property for FlexCAN controller @ 2015-05-14 11:33 ` Bhupesh Sharma 0 siblings, 0 replies; 41+ messages in thread From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw) To: linux-arm-kernel This patch adds 'endianess' as the optional-property for describing the FlexCAN controller present on various FSL platforms. Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> --- .../devicetree/bindings/net/can/fsl-flexcan.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt index 56d6cc3..0d5e668 100644 --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt @@ -18,6 +18,10 @@ Optional properties: - xceiver-supply: Regulator that powers the CAN transceiver +- little-endian: If the FlexCAN IP on this SoC is little-endian, use + this property. By default it is assumed that the IP + is big-endian + Example: can at 1c000 { -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 2/5] arm/dts: Add nodes for flexcan devices present on LS1021A SoC 2015-05-14 11:33 ` Bhupesh Sharma @ 2015-05-14 11:33 ` Bhupesh Sharma -1 siblings, 0 replies; 41+ messages in thread From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw) To: arnd, linux-can, mkl Cc: bhupesh.linux, bhupesh.sharma, Sakar.Arora, linux-arm-kernel From: Sakar Arora <Sakar.Arora@freescale.com> This patch adds the device nodes for flexcan controller(s) present on LS1021A SoC. Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> --- arch/arm/boot/dts/ls1021a-qds.dts | 8 ++++++++ arch/arm/boot/dts/ls1021a.dtsi | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/arch/arm/boot/dts/ls1021a-qds.dts b/arch/arm/boot/dts/ls1021a-qds.dts index 9c5e16b..1641a04 100644 --- a/arch/arm/boot/dts/ls1021a-qds.dts +++ b/arch/arm/boot/dts/ls1021a-qds.dts @@ -238,3 +238,11 @@ &uart1 { status = "okay"; }; + +&can0 { + status = "okay"; +}; + +&can1 { + status = "okay"; +}; diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi index c70bb27..3fb7b52 100644 --- a/arch/arm/boot/dts/ls1021a.dtsi +++ b/arch/arm/boot/dts/ls1021a.dtsi @@ -332,6 +332,24 @@ status = "disabled"; }; + can0: can@2a70000 { + compatible = "fsl,ls1021a-flexcan"; + reg = <0x0 0x2a70000 0x0 0x1000>; + interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&platform_clk 1>, <&platform_clk 1>;; + clock-names = "ipg", "per"; + little-endian; + }; + + can1: can@2a80000 { + compatible = "fsl,ls1021a-flexcan"; + reg = <0x0 0x2a80000 0x0 0x1000>; + interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&platform_clk 1>, <&platform_clk 1>;; + clock-names = "ipg", "per"; + little-endian; + }; + wdog0: watchdog@2ad0000 { compatible = "fsl,imx21-wdt"; reg = <0x0 0x2ad0000 0x0 0x10000>; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 2/5] arm/dts: Add nodes for flexcan devices present on LS1021A SoC @ 2015-05-14 11:33 ` Bhupesh Sharma 0 siblings, 0 replies; 41+ messages in thread From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw) To: linux-arm-kernel From: Sakar Arora <Sakar.Arora@freescale.com> This patch adds the device nodes for flexcan controller(s) present on LS1021A SoC. Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> --- arch/arm/boot/dts/ls1021a-qds.dts | 8 ++++++++ arch/arm/boot/dts/ls1021a.dtsi | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/arch/arm/boot/dts/ls1021a-qds.dts b/arch/arm/boot/dts/ls1021a-qds.dts index 9c5e16b..1641a04 100644 --- a/arch/arm/boot/dts/ls1021a-qds.dts +++ b/arch/arm/boot/dts/ls1021a-qds.dts @@ -238,3 +238,11 @@ &uart1 { status = "okay"; }; + +&can0 { + status = "okay"; +}; + +&can1 { + status = "okay"; +}; diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi index c70bb27..3fb7b52 100644 --- a/arch/arm/boot/dts/ls1021a.dtsi +++ b/arch/arm/boot/dts/ls1021a.dtsi @@ -332,6 +332,24 @@ status = "disabled"; }; + can0: can at 2a70000 { + compatible = "fsl,ls1021a-flexcan"; + reg = <0x0 0x2a70000 0x0 0x1000>; + interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&platform_clk 1>, <&platform_clk 1>;; + clock-names = "ipg", "per"; + little-endian; + }; + + can1: can at 2a80000 { + compatible = "fsl,ls1021a-flexcan"; + reg = <0x0 0x2a80000 0x0 0x1000>; + interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&platform_clk 1>, <&platform_clk 1>;; + clock-names = "ipg", "per"; + little-endian; + }; + wdog0: watchdog at 2ad0000 { compatible = "fsl,imx21-wdt"; reg = <0x0 0x2ad0000 0x0 0x10000>; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 3/5] can: flexcan: Add ls1021a flexcan device entry 2015-05-14 11:33 ` Bhupesh Sharma @ 2015-05-14 11:33 ` Bhupesh Sharma -1 siblings, 0 replies; 41+ messages in thread From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw) To: arnd, linux-can, mkl Cc: bhupesh.linux, bhupesh.sharma, Sakar.Arora, linux-arm-kernel This patch adds ls1021a flexcan device entry to the flexcan driver code. Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> --- drivers/net/can/flexcan.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index ad0a7e8..deaa24e 100644 --- a/drivers/net/can/flexcan.c +++ b/drivers/net/can/flexcan.c @@ -267,10 +267,26 @@ static struct flexcan_devtype_data fsl_imx28_devtype_data; static struct flexcan_devtype_data fsl_imx6q_devtype_data = { .features = FLEXCAN_HAS_V10_FEATURES, }; + static struct flexcan_devtype_data fsl_vf610_devtype_data = { .features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES, }; +/* LS1021A-Rev1 has a broken RX-FIFO support. So only legacy RX message-buffers + * work here. + */ +static struct flexcan_devtype_data fsl_ls1021a_devtype_data = { + .features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES | + FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT, +}; + +/* LS1021A-Rev2 has functional RX-FIFO mode, so no need to fall back to + * the legacy mode. + */ +static struct flexcan_devtype_data fsl_ls1021a_r2_devtype_data = { + .features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES, +}; + static const struct can_bittiming_const flexcan_bittiming_const = { .name = DRV_NAME, .tseg1_min = 4, @@ -1139,6 +1155,10 @@ static void unregister_flexcandev(struct net_device *dev) static const struct of_device_id flexcan_of_match[] = { { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, }, { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, }, + { .compatible = "fsl,ls1021a-flexcan", + .data = &fsl_ls1021a_devtype_data, }, + { .compatible = "fsl,ls1021ar2-flexcan", + .data = &fsl_ls1021a_r2_devtype_data, }, { .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, }, { .compatible = "fsl,vf610-flexcan", .data = &fsl_vf610_devtype_data, }, { /* sentinel */ }, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 3/5] can: flexcan: Add ls1021a flexcan device entry @ 2015-05-14 11:33 ` Bhupesh Sharma 0 siblings, 0 replies; 41+ messages in thread From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw) To: linux-arm-kernel This patch adds ls1021a flexcan device entry to the flexcan driver code. Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> --- drivers/net/can/flexcan.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index ad0a7e8..deaa24e 100644 --- a/drivers/net/can/flexcan.c +++ b/drivers/net/can/flexcan.c @@ -267,10 +267,26 @@ static struct flexcan_devtype_data fsl_imx28_devtype_data; static struct flexcan_devtype_data fsl_imx6q_devtype_data = { .features = FLEXCAN_HAS_V10_FEATURES, }; + static struct flexcan_devtype_data fsl_vf610_devtype_data = { .features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES, }; +/* LS1021A-Rev1 has a broken RX-FIFO support. So only legacy RX message-buffers + * work here. + */ +static struct flexcan_devtype_data fsl_ls1021a_devtype_data = { + .features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES | + FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT, +}; + +/* LS1021A-Rev2 has functional RX-FIFO mode, so no need to fall back to + * the legacy mode. + */ +static struct flexcan_devtype_data fsl_ls1021a_r2_devtype_data = { + .features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES, +}; + static const struct can_bittiming_const flexcan_bittiming_const = { .name = DRV_NAME, .tseg1_min = 4, @@ -1139,6 +1155,10 @@ static void unregister_flexcandev(struct net_device *dev) static const struct of_device_id flexcan_of_match[] = { { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, }, { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, }, + { .compatible = "fsl,ls1021a-flexcan", + .data = &fsl_ls1021a_devtype_data, }, + { .compatible = "fsl,ls1021ar2-flexcan", + .data = &fsl_ls1021a_r2_devtype_data, }, { .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, }, { .compatible = "fsl,vf610-flexcan", .data = &fsl_vf610_devtype_data, }, { /* sentinel */ }, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH v2 3/5] can: flexcan: Add ls1021a flexcan device entry 2015-05-14 11:33 ` Bhupesh Sharma @ 2015-05-14 15:38 ` Marc Kleine-Budde -1 siblings, 0 replies; 41+ messages in thread From: Marc Kleine-Budde @ 2015-05-14 15:38 UTC (permalink / raw) To: Bhupesh Sharma, arnd, linux-can Cc: bhupesh.linux, Sakar.Arora, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 1496 bytes --] On 05/14/2015 01:33 PM, Bhupesh Sharma wrote: > This patch adds ls1021a flexcan device entry to the flexcan driver code. > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> > --- > drivers/net/can/flexcan.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > index ad0a7e8..deaa24e 100644 > --- a/drivers/net/can/flexcan.c > +++ b/drivers/net/can/flexcan.c > @@ -267,10 +267,26 @@ static struct flexcan_devtype_data fsl_imx28_devtype_data; > static struct flexcan_devtype_data fsl_imx6q_devtype_data = { > .features = FLEXCAN_HAS_V10_FEATURES, > }; > + > static struct flexcan_devtype_data fsl_vf610_devtype_data = { > .features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES, > }; > > +/* LS1021A-Rev1 has a broken RX-FIFO support. So only legacy RX message-buffers > + * work here. > + */ > +static struct flexcan_devtype_data fsl_ls1021a_devtype_data = { > + .features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES | > + FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This will probably not compile. 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: 801 bytes --] ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 3/5] can: flexcan: Add ls1021a flexcan device entry @ 2015-05-14 15:38 ` Marc Kleine-Budde 0 siblings, 0 replies; 41+ messages in thread From: Marc Kleine-Budde @ 2015-05-14 15:38 UTC (permalink / raw) To: linux-arm-kernel On 05/14/2015 01:33 PM, Bhupesh Sharma wrote: > This patch adds ls1021a flexcan device entry to the flexcan driver code. > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> > --- > drivers/net/can/flexcan.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > index ad0a7e8..deaa24e 100644 > --- a/drivers/net/can/flexcan.c > +++ b/drivers/net/can/flexcan.c > @@ -267,10 +267,26 @@ static struct flexcan_devtype_data fsl_imx28_devtype_data; > static struct flexcan_devtype_data fsl_imx6q_devtype_data = { > .features = FLEXCAN_HAS_V10_FEATURES, > }; > + > static struct flexcan_devtype_data fsl_vf610_devtype_data = { > .features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES, > }; > > +/* LS1021A-Rev1 has a broken RX-FIFO support. So only legacy RX message-buffers > + * work here. > + */ > +static struct flexcan_devtype_data fsl_ls1021a_devtype_data = { > + .features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES | > + FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This will probably not compile. 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 | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: OpenPGP digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150514/f82e8ded/attachment.sig> ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 4/5] can: flexcan: Remodel FlexCAN register r/w APIs for BE instances 2015-05-14 11:33 ` Bhupesh Sharma @ 2015-05-14 11:33 ` Bhupesh Sharma -1 siblings, 0 replies; 41+ messages in thread From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw) To: arnd, linux-can, mkl Cc: bhupesh.sharma, bhupesh.linux, linux-arm-kernel, Sakar.Arora From: Sakar Arora <Sakar.Arora@freescale.com> The FlexCAN IP on certain SoCs like (Freescale's LS1021A) is modelled in a big-endian fashion, i.e. the registers and the message buffers are organized in a BE way. More details about the LS1021A SoC can be seen here: http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=LS1021A&nodeId=018rH325E4017B# This patch ensures that the register read/write APIs are remodelled to address such cases, while ensuring that existing platforms (where the FlexCAN IP was modelled in LE way) do not break. Tested on LS1021A-QDS board. Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> --- drivers/net/can/flexcan.c | 208 ++++++++++++++++++++++++++------------------- 1 file changed, 119 insertions(+), 89 deletions(-) diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index deaa24e..b0222ae 100644 --- a/drivers/net/can/flexcan.c +++ b/drivers/net/can/flexcan.c @@ -258,6 +258,10 @@ struct flexcan_priv { struct flexcan_platform_data *pdata; const struct flexcan_devtype_data *devtype_data; struct regulator *reg_xceiver; + + /* Read and Write APIs */ + u32 (*read)(void __iomem *addr); + void (*write)(u32 val, void __iomem *addr); }; static struct flexcan_devtype_data fsl_p1010_devtype_data = { @@ -300,32 +304,38 @@ static const struct can_bittiming_const flexcan_bittiming_const = { }; /* - * Abstract off the read/write for arm versus ppc. This - * assumes that PPC uses big-endian registers and everything - * else uses little-endian registers, independent of CPU - * endianess. + * FlexCAN module is essentially modelled as a little-endian IP in most + * SoCs, i.e the registers as well as the message buffer areas are + * implemented in a little-endian fashion. + * + * However there are some SoCs (e.g. LS1021A) which implement the FlexCAN + * module in a big-endian fashion (i.e the registers as well as the + * message buffer areas are implemented in a big-endian way). + * + * In addition, the FlexCAN module can be found on SoCs having ARM or + * PPC cores. So, we need to abstract off the register read/write + * functions, ensuring that these cater to all the combinations of module + * endianness and underlying CPU endianness. */ -#if defined(CONFIG_PPC) -static inline u32 flexcan_read(void __iomem *addr) +static inline u32 flexcan_read_le(void __iomem *addr) { - return in_be32(addr); + return ioread32(addr); } -static inline void flexcan_write(u32 val, void __iomem *addr) +static inline void flexcan_write_le(u32 val, void __iomem *addr) { - out_be32(addr, val); + iowrite32(val, addr); } -#else -static inline u32 flexcan_read(void __iomem *addr) + +static inline u32 flexcan_read_be(void __iomem *addr) { - return readl(addr); + return ioread32be(addr); } -static inline void flexcan_write(u32 val, void __iomem *addr) +static inline void flexcan_write_be(u32 val, void __iomem *addr) { - writel(val, addr); + iowrite32be(val, addr); } -#endif static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv) { @@ -356,14 +366,14 @@ static int flexcan_chip_enable(struct flexcan_priv *priv) unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; u32 reg; - reg = flexcan_read(®s->mcr); + reg = priv->read(®s->mcr); reg &= ~FLEXCAN_MCR_MDIS; - flexcan_write(reg, ®s->mcr); + priv->write(reg, ®s->mcr); - while (timeout-- && (flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) + while (timeout-- && (priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) udelay(10); - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK) + if (priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK) return -ETIMEDOUT; return 0; @@ -375,14 +385,14 @@ static int flexcan_chip_disable(struct flexcan_priv *priv) unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; u32 reg; - reg = flexcan_read(®s->mcr); + reg = priv->read(®s->mcr); reg |= FLEXCAN_MCR_MDIS; - flexcan_write(reg, ®s->mcr); + priv->write(reg, ®s->mcr); - while (timeout-- && !(flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) + while (timeout-- && !(priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) udelay(10); - if (!(flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) + if (!(priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) return -ETIMEDOUT; return 0; @@ -394,14 +404,14 @@ static int flexcan_chip_freeze(struct flexcan_priv *priv) unsigned int timeout = 1000 * 1000 * 10 / priv->can.bittiming.bitrate; u32 reg; - reg = flexcan_read(®s->mcr); + reg = priv->read(®s->mcr); reg |= FLEXCAN_MCR_HALT; - flexcan_write(reg, ®s->mcr); + priv->write(reg, ®s->mcr); - while (timeout-- && !(flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) + while (timeout-- && !(priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) udelay(100); - if (!(flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) + if (!(priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) return -ETIMEDOUT; return 0; @@ -413,14 +423,14 @@ static int flexcan_chip_unfreeze(struct flexcan_priv *priv) unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; u32 reg; - reg = flexcan_read(®s->mcr); + reg = priv->read(®s->mcr); reg &= ~FLEXCAN_MCR_HALT; - flexcan_write(reg, ®s->mcr); + priv->write(reg, ®s->mcr); - while (timeout-- && (flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) + while (timeout-- && (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) udelay(10); - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK) + if (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK) return -ETIMEDOUT; return 0; @@ -431,23 +441,22 @@ static int flexcan_chip_softreset(struct flexcan_priv *priv) struct flexcan_regs __iomem *regs = priv->base; unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; - flexcan_write(FLEXCAN_MCR_SOFTRST, ®s->mcr); - while (timeout-- && (flexcan_read(®s->mcr) & FLEXCAN_MCR_SOFTRST)) + priv->write(FLEXCAN_MCR_SOFTRST, ®s->mcr); + while (timeout-- && (priv->read(®s->mcr) & FLEXCAN_MCR_SOFTRST)) udelay(10); - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_SOFTRST) + if (priv->read(®s->mcr) & FLEXCAN_MCR_SOFTRST) return -ETIMEDOUT; return 0; } - static int __flexcan_get_berr_counter(const struct net_device *dev, struct can_berr_counter *bec) { const struct flexcan_priv *priv = netdev_priv(dev); struct flexcan_regs __iomem *regs = priv->base; - u32 reg = flexcan_read(®s->ecr); + u32 reg = priv->read(®s->ecr); bec->txerr = (reg >> 0) & 0xff; bec->rxerr = (reg >> 8) & 0xff; @@ -503,24 +512,24 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev) if (cf->can_dlc > 0) { u32 data = be32_to_cpup((__be32 *)&cf->data[0]); - flexcan_write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[0]); + priv->write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[0]); } if (cf->can_dlc > 3) { u32 data = be32_to_cpup((__be32 *)&cf->data[4]); - flexcan_write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[1]); + priv->write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[1]); } can_put_echo_skb(skb, dev, 0); - flexcan_write(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id); - flexcan_write(ctrl, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); + priv->write(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id); + priv->write(ctrl, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); /* Errata ERR005829 step8: * Write twice INACTIVE(0x8) code to first MB. */ - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); return NETDEV_TX_OK; @@ -645,8 +654,8 @@ static void flexcan_read_fifo(const struct net_device *dev, struct flexcan_mb __iomem *mb = ®s->cantxfg[0]; u32 reg_ctrl, reg_id; - reg_ctrl = flexcan_read(&mb->can_ctrl); - reg_id = flexcan_read(&mb->can_id); + reg_ctrl = priv->read(&mb->can_ctrl); + reg_id = priv->read(&mb->can_id); if (reg_ctrl & FLEXCAN_MB_CNT_IDE) cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG; else @@ -656,12 +665,12 @@ static void flexcan_read_fifo(const struct net_device *dev, cf->can_id |= CAN_RTR_FLAG; cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf); - *(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb->data[0])); - *(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb->data[1])); + *(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb->data[0])); + *(__be32 *)(cf->data + 4) = cpu_to_be32(priv->read(&mb->data[1])); /* mark as read */ - flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); - flexcan_read(®s->timer); + priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); + priv->read(®s->timer); } static int flexcan_read_frame(struct net_device *dev) @@ -699,17 +708,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota) * The error bits are cleared on read, * use saved value from irq handler. */ - reg_esr = flexcan_read(®s->esr) | priv->reg_esr; + reg_esr = priv->read(®s->esr) | priv->reg_esr; /* handle state changes */ work_done += flexcan_poll_state(dev, reg_esr); /* handle RX-FIFO */ - reg_iflag1 = flexcan_read(®s->iflag1); + reg_iflag1 = priv->read(®s->iflag1); while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE && work_done < quota) { work_done += flexcan_read_frame(dev); - reg_iflag1 = flexcan_read(®s->iflag1); + reg_iflag1 = priv->read(®s->iflag1); } /* report bus errors */ @@ -719,8 +728,8 @@ static int flexcan_poll(struct napi_struct *napi, int quota) if (work_done < quota) { napi_complete(napi); /* enable IRQs */ - flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); - flexcan_write(priv->reg_ctrl_default, ®s->ctrl); + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); + priv->write(priv->reg_ctrl_default, ®s->ctrl); } return work_done; @@ -734,11 +743,11 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) struct flexcan_regs __iomem *regs = priv->base; u32 reg_iflag1, reg_esr; - reg_iflag1 = flexcan_read(®s->iflag1); - reg_esr = flexcan_read(®s->esr); + reg_iflag1 = priv->read(®s->iflag1); + reg_esr = priv->read(®s->esr); /* ACK all bus error and state change IRQ sources */ if (reg_esr & FLEXCAN_ESR_ALL_INT) - flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr); + priv->write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr); /* * schedule NAPI in case of: @@ -754,16 +763,16 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) * save them for later use. */ priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS; - flexcan_write(FLEXCAN_IFLAG_DEFAULT & + priv->write(FLEXCAN_IFLAG_DEFAULT & ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->imask1); - flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, + priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, ®s->ctrl); napi_schedule(&priv->napi); } /* FIFO overflow */ if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) { - flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, ®s->iflag1); + priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, ®s->iflag1); dev->stats.rx_over_errors++; dev->stats.rx_errors++; } @@ -774,9 +783,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) stats->tx_packets++; can_led_event(dev, CAN_LED_EVENT_TX); /* after sending a RTR frame mailbox is in RX mode */ - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); - flexcan_write((1 << FLEXCAN_TX_BUF_ID), ®s->iflag1); + priv->write((1 << FLEXCAN_TX_BUF_ID), ®s->iflag1); netif_wake_queue(dev); } @@ -790,7 +799,7 @@ static void flexcan_set_bittiming(struct net_device *dev) struct flexcan_regs __iomem *regs = priv->base; u32 reg; - reg = flexcan_read(®s->ctrl); + reg = priv->read(®s->ctrl); reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) | FLEXCAN_CTRL_RJW(0x3) | FLEXCAN_CTRL_PSEG1(0x7) | @@ -814,11 +823,11 @@ static void flexcan_set_bittiming(struct net_device *dev) reg |= FLEXCAN_CTRL_SMP; netdev_info(dev, "writing ctrl=0x%08x\n", reg); - flexcan_write(reg, ®s->ctrl); + priv->write(reg, ®s->ctrl); /* print chip status */ netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__, - flexcan_read(®s->mcr), flexcan_read(®s->ctrl)); + priv->read(®s->mcr), priv->read(®s->ctrl)); } /* @@ -858,14 +867,14 @@ static int flexcan_chip_start(struct net_device *dev) * disable local echo * */ - reg_mcr = flexcan_read(®s->mcr); + reg_mcr = priv->read(®s->mcr); reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff); reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT | FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN | FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS | FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID); netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr); - flexcan_write(reg_mcr, ®s->mcr); + priv->write(reg_mcr, ®s->mcr); /* * CTRL @@ -879,7 +888,7 @@ static int flexcan_chip_start(struct net_device *dev) * enable bus off interrupt * (== FLEXCAN_CTRL_ERR_STATE) */ - reg_ctrl = flexcan_read(®s->ctrl); + reg_ctrl = priv->read(®s->ctrl); reg_ctrl &= ~FLEXCAN_CTRL_TSYN; reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF | FLEXCAN_CTRL_ERR_STATE; @@ -897,29 +906,29 @@ static int flexcan_chip_start(struct net_device *dev) /* save for later use */ priv->reg_ctrl_default = reg_ctrl; netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl); - flexcan_write(reg_ctrl, ®s->ctrl); + priv->write(reg_ctrl, ®s->ctrl); /* clear and invalidate all mailboxes first */ for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->cantxfg); i++) { - flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE, + priv->write(FLEXCAN_MB_CODE_RX_INACTIVE, ®s->cantxfg[i].can_ctrl); } /* Errata ERR005829: mark first TX mailbox as INACTIVE */ - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); /* mark TX mailbox as INACTIVE */ - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); /* acceptance mask/acceptance code (accept everything) */ - flexcan_write(0x0, ®s->rxgmask); - flexcan_write(0x0, ®s->rx14mask); - flexcan_write(0x0, ®s->rx15mask); + priv->write(0x0, ®s->rxgmask); + priv->write(0x0, ®s->rx14mask); + priv->write(0x0, ®s->rx15mask); if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) - flexcan_write(0x0, ®s->rxfgmask); + priv->write(0x0, ®s->rxfgmask); /* * On Vybrid, disable memory error detection interrupts @@ -934,16 +943,16 @@ static int flexcan_chip_start(struct net_device *dev) * and Correction of Memory Errors" to write to * MECR register */ - reg_crl2 = flexcan_read(®s->crl2); + reg_crl2 = priv->read(®s->crl2); reg_crl2 |= FLEXCAN_CRL2_ECRWRE; - flexcan_write(reg_crl2, ®s->crl2); + priv->write(reg_crl2, ®s->crl2); - reg_mecr = flexcan_read(®s->mecr); + reg_mecr = priv->read(®s->mecr); reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS; - flexcan_write(reg_mecr, ®s->mecr); + priv->write(reg_mecr, ®s->mecr); reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ | FLEXCAN_MECR_HANCEI_MSK | FLEXCAN_MECR_FANCEI_MSK); - flexcan_write(reg_mecr, ®s->mecr); + priv->write(reg_mecr, ®s->mecr); } err = flexcan_transceiver_enable(priv); @@ -958,11 +967,11 @@ static int flexcan_chip_start(struct net_device *dev) priv->can.state = CAN_STATE_ERROR_ACTIVE; /* enable FIFO interrupts */ - flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); /* print chip status */ netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__, - flexcan_read(®s->mcr), flexcan_read(®s->ctrl)); + priv->read(®s->mcr), priv->read(®s->ctrl)); return 0; @@ -989,8 +998,8 @@ static void flexcan_chip_stop(struct net_device *dev) flexcan_chip_disable(priv); /* Disable all interrupts */ - flexcan_write(0, ®s->imask1); - flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, + priv->write(0, ®s->imask1); + priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, ®s->ctrl); flexcan_transceiver_disable(priv); @@ -1108,26 +1117,26 @@ static int register_flexcandev(struct net_device *dev) err = flexcan_chip_disable(priv); if (err) goto out_disable_per; - reg = flexcan_read(®s->ctrl); + reg = priv->read(®s->ctrl); reg |= FLEXCAN_CTRL_CLK_SRC; - flexcan_write(reg, ®s->ctrl); + priv->write(reg, ®s->ctrl); err = flexcan_chip_enable(priv); if (err) goto out_chip_disable; /* set freeze, halt and activate FIFO, restrict register access */ - reg = flexcan_read(®s->mcr); + reg = priv->read(®s->mcr); reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV; - flexcan_write(reg, ®s->mcr); + priv->write(reg, ®s->mcr); /* * Currently we only support newer versions of this core * featuring a RX FIFO. Older cores found on some Coldfire * derivates are not yet supported. */ - reg = flexcan_read(®s->mcr); + reg = priv->read(®s->mcr); if (!(reg & FLEXCAN_MCR_FEN)) { netdev_err(dev, "Could not enable RX FIFO, unsupported core\n"); err = -ENODEV; @@ -1183,6 +1192,8 @@ static int flexcan_probe(struct platform_device *pdev) void __iomem *base; int err, irq; u32 clock_freq = 0; + /* Default case for most ARM based FSL SoC having BE FlexCAN IP */ + bool core_is_little = true, module_is_little = false; reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver"); if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER) @@ -1237,6 +1248,25 @@ static int flexcan_probe(struct platform_device *pdev) dev->flags |= IFF_ECHO; priv = netdev_priv(dev); + + if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) + core_is_little = false; + + if (of_property_read_bool(dev->dev.of_node, "little-endian")) + module_is_little = true; + + if ((core_is_little && module_is_little) || + (!core_is_little && !module_is_little)) { + priv->read = flexcan_read_le; + priv->write = flexcan_write_le; + } + + if ((!core_is_little && module_is_little) || + (core_is_little && !module_is_little)) { + priv->read = flexcan_read_be; + priv->write = flexcan_write_be; + } + priv->can.clock.freq = clock_freq; priv->can.bittiming_const = &flexcan_bittiming_const; priv->can.do_set_mode = flexcan_set_mode; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 4/5] can: flexcan: Remodel FlexCAN register r/w APIs for BE instances @ 2015-05-14 11:33 ` Bhupesh Sharma 0 siblings, 0 replies; 41+ messages in thread From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw) To: linux-arm-kernel From: Sakar Arora <Sakar.Arora@freescale.com> The FlexCAN IP on certain SoCs like (Freescale's LS1021A) is modelled in a big-endian fashion, i.e. the registers and the message buffers are organized in a BE way. More details about the LS1021A SoC can be seen here: http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=LS1021A&nodeId=018rH325E4017B# This patch ensures that the register read/write APIs are remodelled to address such cases, while ensuring that existing platforms (where the FlexCAN IP was modelled in LE way) do not break. Tested on LS1021A-QDS board. Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> --- drivers/net/can/flexcan.c | 208 ++++++++++++++++++++++++++------------------- 1 file changed, 119 insertions(+), 89 deletions(-) diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index deaa24e..b0222ae 100644 --- a/drivers/net/can/flexcan.c +++ b/drivers/net/can/flexcan.c @@ -258,6 +258,10 @@ struct flexcan_priv { struct flexcan_platform_data *pdata; const struct flexcan_devtype_data *devtype_data; struct regulator *reg_xceiver; + + /* Read and Write APIs */ + u32 (*read)(void __iomem *addr); + void (*write)(u32 val, void __iomem *addr); }; static struct flexcan_devtype_data fsl_p1010_devtype_data = { @@ -300,32 +304,38 @@ static const struct can_bittiming_const flexcan_bittiming_const = { }; /* - * Abstract off the read/write for arm versus ppc. This - * assumes that PPC uses big-endian registers and everything - * else uses little-endian registers, independent of CPU - * endianess. + * FlexCAN module is essentially modelled as a little-endian IP in most + * SoCs, i.e the registers as well as the message buffer areas are + * implemented in a little-endian fashion. + * + * However there are some SoCs (e.g. LS1021A) which implement the FlexCAN + * module in a big-endian fashion (i.e the registers as well as the + * message buffer areas are implemented in a big-endian way). + * + * In addition, the FlexCAN module can be found on SoCs having ARM or + * PPC cores. So, we need to abstract off the register read/write + * functions, ensuring that these cater to all the combinations of module + * endianness and underlying CPU endianness. */ -#if defined(CONFIG_PPC) -static inline u32 flexcan_read(void __iomem *addr) +static inline u32 flexcan_read_le(void __iomem *addr) { - return in_be32(addr); + return ioread32(addr); } -static inline void flexcan_write(u32 val, void __iomem *addr) +static inline void flexcan_write_le(u32 val, void __iomem *addr) { - out_be32(addr, val); + iowrite32(val, addr); } -#else -static inline u32 flexcan_read(void __iomem *addr) + +static inline u32 flexcan_read_be(void __iomem *addr) { - return readl(addr); + return ioread32be(addr); } -static inline void flexcan_write(u32 val, void __iomem *addr) +static inline void flexcan_write_be(u32 val, void __iomem *addr) { - writel(val, addr); + iowrite32be(val, addr); } -#endif static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv) { @@ -356,14 +366,14 @@ static int flexcan_chip_enable(struct flexcan_priv *priv) unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; u32 reg; - reg = flexcan_read(®s->mcr); + reg = priv->read(®s->mcr); reg &= ~FLEXCAN_MCR_MDIS; - flexcan_write(reg, ®s->mcr); + priv->write(reg, ®s->mcr); - while (timeout-- && (flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) + while (timeout-- && (priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) udelay(10); - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK) + if (priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK) return -ETIMEDOUT; return 0; @@ -375,14 +385,14 @@ static int flexcan_chip_disable(struct flexcan_priv *priv) unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; u32 reg; - reg = flexcan_read(®s->mcr); + reg = priv->read(®s->mcr); reg |= FLEXCAN_MCR_MDIS; - flexcan_write(reg, ®s->mcr); + priv->write(reg, ®s->mcr); - while (timeout-- && !(flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) + while (timeout-- && !(priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) udelay(10); - if (!(flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) + if (!(priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) return -ETIMEDOUT; return 0; @@ -394,14 +404,14 @@ static int flexcan_chip_freeze(struct flexcan_priv *priv) unsigned int timeout = 1000 * 1000 * 10 / priv->can.bittiming.bitrate; u32 reg; - reg = flexcan_read(®s->mcr); + reg = priv->read(®s->mcr); reg |= FLEXCAN_MCR_HALT; - flexcan_write(reg, ®s->mcr); + priv->write(reg, ®s->mcr); - while (timeout-- && !(flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) + while (timeout-- && !(priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) udelay(100); - if (!(flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) + if (!(priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) return -ETIMEDOUT; return 0; @@ -413,14 +423,14 @@ static int flexcan_chip_unfreeze(struct flexcan_priv *priv) unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; u32 reg; - reg = flexcan_read(®s->mcr); + reg = priv->read(®s->mcr); reg &= ~FLEXCAN_MCR_HALT; - flexcan_write(reg, ®s->mcr); + priv->write(reg, ®s->mcr); - while (timeout-- && (flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) + while (timeout-- && (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) udelay(10); - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK) + if (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK) return -ETIMEDOUT; return 0; @@ -431,23 +441,22 @@ static int flexcan_chip_softreset(struct flexcan_priv *priv) struct flexcan_regs __iomem *regs = priv->base; unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; - flexcan_write(FLEXCAN_MCR_SOFTRST, ®s->mcr); - while (timeout-- && (flexcan_read(®s->mcr) & FLEXCAN_MCR_SOFTRST)) + priv->write(FLEXCAN_MCR_SOFTRST, ®s->mcr); + while (timeout-- && (priv->read(®s->mcr) & FLEXCAN_MCR_SOFTRST)) udelay(10); - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_SOFTRST) + if (priv->read(®s->mcr) & FLEXCAN_MCR_SOFTRST) return -ETIMEDOUT; return 0; } - static int __flexcan_get_berr_counter(const struct net_device *dev, struct can_berr_counter *bec) { const struct flexcan_priv *priv = netdev_priv(dev); struct flexcan_regs __iomem *regs = priv->base; - u32 reg = flexcan_read(®s->ecr); + u32 reg = priv->read(®s->ecr); bec->txerr = (reg >> 0) & 0xff; bec->rxerr = (reg >> 8) & 0xff; @@ -503,24 +512,24 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev) if (cf->can_dlc > 0) { u32 data = be32_to_cpup((__be32 *)&cf->data[0]); - flexcan_write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[0]); + priv->write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[0]); } if (cf->can_dlc > 3) { u32 data = be32_to_cpup((__be32 *)&cf->data[4]); - flexcan_write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[1]); + priv->write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[1]); } can_put_echo_skb(skb, dev, 0); - flexcan_write(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id); - flexcan_write(ctrl, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); + priv->write(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id); + priv->write(ctrl, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); /* Errata ERR005829 step8: * Write twice INACTIVE(0x8) code to first MB. */ - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); return NETDEV_TX_OK; @@ -645,8 +654,8 @@ static void flexcan_read_fifo(const struct net_device *dev, struct flexcan_mb __iomem *mb = ®s->cantxfg[0]; u32 reg_ctrl, reg_id; - reg_ctrl = flexcan_read(&mb->can_ctrl); - reg_id = flexcan_read(&mb->can_id); + reg_ctrl = priv->read(&mb->can_ctrl); + reg_id = priv->read(&mb->can_id); if (reg_ctrl & FLEXCAN_MB_CNT_IDE) cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG; else @@ -656,12 +665,12 @@ static void flexcan_read_fifo(const struct net_device *dev, cf->can_id |= CAN_RTR_FLAG; cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf); - *(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb->data[0])); - *(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb->data[1])); + *(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb->data[0])); + *(__be32 *)(cf->data + 4) = cpu_to_be32(priv->read(&mb->data[1])); /* mark as read */ - flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); - flexcan_read(®s->timer); + priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); + priv->read(®s->timer); } static int flexcan_read_frame(struct net_device *dev) @@ -699,17 +708,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota) * The error bits are cleared on read, * use saved value from irq handler. */ - reg_esr = flexcan_read(®s->esr) | priv->reg_esr; + reg_esr = priv->read(®s->esr) | priv->reg_esr; /* handle state changes */ work_done += flexcan_poll_state(dev, reg_esr); /* handle RX-FIFO */ - reg_iflag1 = flexcan_read(®s->iflag1); + reg_iflag1 = priv->read(®s->iflag1); while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE && work_done < quota) { work_done += flexcan_read_frame(dev); - reg_iflag1 = flexcan_read(®s->iflag1); + reg_iflag1 = priv->read(®s->iflag1); } /* report bus errors */ @@ -719,8 +728,8 @@ static int flexcan_poll(struct napi_struct *napi, int quota) if (work_done < quota) { napi_complete(napi); /* enable IRQs */ - flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); - flexcan_write(priv->reg_ctrl_default, ®s->ctrl); + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); + priv->write(priv->reg_ctrl_default, ®s->ctrl); } return work_done; @@ -734,11 +743,11 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) struct flexcan_regs __iomem *regs = priv->base; u32 reg_iflag1, reg_esr; - reg_iflag1 = flexcan_read(®s->iflag1); - reg_esr = flexcan_read(®s->esr); + reg_iflag1 = priv->read(®s->iflag1); + reg_esr = priv->read(®s->esr); /* ACK all bus error and state change IRQ sources */ if (reg_esr & FLEXCAN_ESR_ALL_INT) - flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr); + priv->write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr); /* * schedule NAPI in case of: @@ -754,16 +763,16 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) * save them for later use. */ priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS; - flexcan_write(FLEXCAN_IFLAG_DEFAULT & + priv->write(FLEXCAN_IFLAG_DEFAULT & ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->imask1); - flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, + priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, ®s->ctrl); napi_schedule(&priv->napi); } /* FIFO overflow */ if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) { - flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, ®s->iflag1); + priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, ®s->iflag1); dev->stats.rx_over_errors++; dev->stats.rx_errors++; } @@ -774,9 +783,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) stats->tx_packets++; can_led_event(dev, CAN_LED_EVENT_TX); /* after sending a RTR frame mailbox is in RX mode */ - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); - flexcan_write((1 << FLEXCAN_TX_BUF_ID), ®s->iflag1); + priv->write((1 << FLEXCAN_TX_BUF_ID), ®s->iflag1); netif_wake_queue(dev); } @@ -790,7 +799,7 @@ static void flexcan_set_bittiming(struct net_device *dev) struct flexcan_regs __iomem *regs = priv->base; u32 reg; - reg = flexcan_read(®s->ctrl); + reg = priv->read(®s->ctrl); reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) | FLEXCAN_CTRL_RJW(0x3) | FLEXCAN_CTRL_PSEG1(0x7) | @@ -814,11 +823,11 @@ static void flexcan_set_bittiming(struct net_device *dev) reg |= FLEXCAN_CTRL_SMP; netdev_info(dev, "writing ctrl=0x%08x\n", reg); - flexcan_write(reg, ®s->ctrl); + priv->write(reg, ®s->ctrl); /* print chip status */ netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__, - flexcan_read(®s->mcr), flexcan_read(®s->ctrl)); + priv->read(®s->mcr), priv->read(®s->ctrl)); } /* @@ -858,14 +867,14 @@ static int flexcan_chip_start(struct net_device *dev) * disable local echo * */ - reg_mcr = flexcan_read(®s->mcr); + reg_mcr = priv->read(®s->mcr); reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff); reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT | FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN | FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS | FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID); netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr); - flexcan_write(reg_mcr, ®s->mcr); + priv->write(reg_mcr, ®s->mcr); /* * CTRL @@ -879,7 +888,7 @@ static int flexcan_chip_start(struct net_device *dev) * enable bus off interrupt * (== FLEXCAN_CTRL_ERR_STATE) */ - reg_ctrl = flexcan_read(®s->ctrl); + reg_ctrl = priv->read(®s->ctrl); reg_ctrl &= ~FLEXCAN_CTRL_TSYN; reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF | FLEXCAN_CTRL_ERR_STATE; @@ -897,29 +906,29 @@ static int flexcan_chip_start(struct net_device *dev) /* save for later use */ priv->reg_ctrl_default = reg_ctrl; netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl); - flexcan_write(reg_ctrl, ®s->ctrl); + priv->write(reg_ctrl, ®s->ctrl); /* clear and invalidate all mailboxes first */ for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->cantxfg); i++) { - flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE, + priv->write(FLEXCAN_MB_CODE_RX_INACTIVE, ®s->cantxfg[i].can_ctrl); } /* Errata ERR005829: mark first TX mailbox as INACTIVE */ - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); /* mark TX mailbox as INACTIVE */ - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); /* acceptance mask/acceptance code (accept everything) */ - flexcan_write(0x0, ®s->rxgmask); - flexcan_write(0x0, ®s->rx14mask); - flexcan_write(0x0, ®s->rx15mask); + priv->write(0x0, ®s->rxgmask); + priv->write(0x0, ®s->rx14mask); + priv->write(0x0, ®s->rx15mask); if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) - flexcan_write(0x0, ®s->rxfgmask); + priv->write(0x0, ®s->rxfgmask); /* * On Vybrid, disable memory error detection interrupts @@ -934,16 +943,16 @@ static int flexcan_chip_start(struct net_device *dev) * and Correction of Memory Errors" to write to * MECR register */ - reg_crl2 = flexcan_read(®s->crl2); + reg_crl2 = priv->read(®s->crl2); reg_crl2 |= FLEXCAN_CRL2_ECRWRE; - flexcan_write(reg_crl2, ®s->crl2); + priv->write(reg_crl2, ®s->crl2); - reg_mecr = flexcan_read(®s->mecr); + reg_mecr = priv->read(®s->mecr); reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS; - flexcan_write(reg_mecr, ®s->mecr); + priv->write(reg_mecr, ®s->mecr); reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ | FLEXCAN_MECR_HANCEI_MSK | FLEXCAN_MECR_FANCEI_MSK); - flexcan_write(reg_mecr, ®s->mecr); + priv->write(reg_mecr, ®s->mecr); } err = flexcan_transceiver_enable(priv); @@ -958,11 +967,11 @@ static int flexcan_chip_start(struct net_device *dev) priv->can.state = CAN_STATE_ERROR_ACTIVE; /* enable FIFO interrupts */ - flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); /* print chip status */ netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__, - flexcan_read(®s->mcr), flexcan_read(®s->ctrl)); + priv->read(®s->mcr), priv->read(®s->ctrl)); return 0; @@ -989,8 +998,8 @@ static void flexcan_chip_stop(struct net_device *dev) flexcan_chip_disable(priv); /* Disable all interrupts */ - flexcan_write(0, ®s->imask1); - flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, + priv->write(0, ®s->imask1); + priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, ®s->ctrl); flexcan_transceiver_disable(priv); @@ -1108,26 +1117,26 @@ static int register_flexcandev(struct net_device *dev) err = flexcan_chip_disable(priv); if (err) goto out_disable_per; - reg = flexcan_read(®s->ctrl); + reg = priv->read(®s->ctrl); reg |= FLEXCAN_CTRL_CLK_SRC; - flexcan_write(reg, ®s->ctrl); + priv->write(reg, ®s->ctrl); err = flexcan_chip_enable(priv); if (err) goto out_chip_disable; /* set freeze, halt and activate FIFO, restrict register access */ - reg = flexcan_read(®s->mcr); + reg = priv->read(®s->mcr); reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV; - flexcan_write(reg, ®s->mcr); + priv->write(reg, ®s->mcr); /* * Currently we only support newer versions of this core * featuring a RX FIFO. Older cores found on some Coldfire * derivates are not yet supported. */ - reg = flexcan_read(®s->mcr); + reg = priv->read(®s->mcr); if (!(reg & FLEXCAN_MCR_FEN)) { netdev_err(dev, "Could not enable RX FIFO, unsupported core\n"); err = -ENODEV; @@ -1183,6 +1192,8 @@ static int flexcan_probe(struct platform_device *pdev) void __iomem *base; int err, irq; u32 clock_freq = 0; + /* Default case for most ARM based FSL SoC having BE FlexCAN IP */ + bool core_is_little = true, module_is_little = false; reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver"); if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER) @@ -1237,6 +1248,25 @@ static int flexcan_probe(struct platform_device *pdev) dev->flags |= IFF_ECHO; priv = netdev_priv(dev); + + if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) + core_is_little = false; + + if (of_property_read_bool(dev->dev.of_node, "little-endian")) + module_is_little = true; + + if ((core_is_little && module_is_little) || + (!core_is_little && !module_is_little)) { + priv->read = flexcan_read_le; + priv->write = flexcan_write_le; + } + + if ((!core_is_little && module_is_little) || + (core_is_little && !module_is_little)) { + priv->read = flexcan_read_be; + priv->write = flexcan_write_be; + } + priv->can.clock.freq = clock_freq; priv->can.bittiming_const = &flexcan_bittiming_const; priv->can.do_set_mode = flexcan_set_mode; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH v2 4/5] can: flexcan: Remodel FlexCAN register r/w APIs for BE instances 2015-05-14 11:33 ` Bhupesh Sharma @ 2015-05-18 16:17 ` Enrico Weigelt, metux IT consult -1 siblings, 0 replies; 41+ messages in thread From: Enrico Weigelt, metux IT consult @ 2015-05-18 16:17 UTC (permalink / raw) To: Bhupesh Sharma, arnd, linux-can, mkl Cc: bhupesh.linux, linux-arm-kernel, Sakar.Arora Am 14.05.2015 um 13:33 schrieb Bhupesh Sharma: > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > index deaa24e..b0222ae 100644 > --- a/drivers/net/can/flexcan.c > +++ b/drivers/net/can/flexcan.c > @@ -258,6 +258,10 @@ struct flexcan_priv { > struct flexcan_platform_data *pdata; > const struct flexcan_devtype_data *devtype_data; > struct regulator *reg_xceiver; > + > + /* Read and Write APIs */ > + u32 (*read)(void __iomem *addr); > + void (*write)(u32 val, void __iomem *addr); Does it *really* need to be a far call ? Why not just give the existing flexcan_read()/flexcan_write() inline's an additional bit to decide whether to swab or not ? > -#if defined(CONFIG_PPC) > -static inline u32 flexcan_read(void __iomem *addr) > +static inline u32 flexcan_read_le(void __iomem *addr) _static inline_ as a callback ? seriously ? > static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv) > { > @@ -356,14 +366,14 @@ static int flexcan_chip_enable(struct flexcan_priv *priv) > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > u32 reg; > > - reg = flexcan_read(®s->mcr); > + reg = priv->read(®s->mcr); > reg &= ~FLEXCAN_MCR_MDIS; > - flexcan_write(reg, ®s->mcr); > + priv->write(reg, ®s->mcr); > > - while (timeout-- && (flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > + while (timeout-- && (priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > udelay(10); > > - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK) > + if (priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK) > return -ETIMEDOUT; > > return 0; > @@ -375,14 +385,14 @@ static int flexcan_chip_disable(struct flexcan_priv *priv) > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > u32 reg; > > - reg = flexcan_read(®s->mcr); > + reg = priv->read(®s->mcr); > reg |= FLEXCAN_MCR_MDIS; > - flexcan_write(reg, ®s->mcr); > + priv->write(reg, ®s->mcr); > > - while (timeout-- && !(flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > + while (timeout-- && !(priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > udelay(10); > > - if (!(flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > + if (!(priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > return -ETIMEDOUT; > > return 0; > @@ -394,14 +404,14 @@ static int flexcan_chip_freeze(struct flexcan_priv *priv) > unsigned int timeout = 1000 * 1000 * 10 / priv->can.bittiming.bitrate; > u32 reg; > > - reg = flexcan_read(®s->mcr); > + reg = priv->read(®s->mcr); > reg |= FLEXCAN_MCR_HALT; > - flexcan_write(reg, ®s->mcr); > + priv->write(reg, ®s->mcr); > > - while (timeout-- && !(flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > + while (timeout-- && !(priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > udelay(100); > > - if (!(flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > + if (!(priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > return -ETIMEDOUT; > > return 0; > @@ -413,14 +423,14 @@ static int flexcan_chip_unfreeze(struct flexcan_priv *priv) > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > u32 reg; > > - reg = flexcan_read(®s->mcr); > + reg = priv->read(®s->mcr); > reg &= ~FLEXCAN_MCR_HALT; > - flexcan_write(reg, ®s->mcr); > + priv->write(reg, ®s->mcr); > > - while (timeout-- && (flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > + while (timeout-- && (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > udelay(10); > > - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK) > + if (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK) > return -ETIMEDOUT; > > return 0; > @@ -431,23 +441,22 @@ static int flexcan_chip_softreset(struct flexcan_priv *priv) > struct flexcan_regs __iomem *regs = priv->base; > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > > - flexcan_write(FLEXCAN_MCR_SOFTRST, ®s->mcr); > - while (timeout-- && (flexcan_read(®s->mcr) & FLEXCAN_MCR_SOFTRST)) > + priv->write(FLEXCAN_MCR_SOFTRST, ®s->mcr); > + while (timeout-- && (priv->read(®s->mcr) & FLEXCAN_MCR_SOFTRST)) > udelay(10); > > - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_SOFTRST) > + if (priv->read(®s->mcr) & FLEXCAN_MCR_SOFTRST) > return -ETIMEDOUT; > > return 0; > } > > - > static int __flexcan_get_berr_counter(const struct net_device *dev, > struct can_berr_counter *bec) > { > const struct flexcan_priv *priv = netdev_priv(dev); > struct flexcan_regs __iomem *regs = priv->base; > - u32 reg = flexcan_read(®s->ecr); > + u32 reg = priv->read(®s->ecr); > > bec->txerr = (reg >> 0) & 0xff; > bec->rxerr = (reg >> 8) & 0xff; > @@ -503,24 +512,24 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev) > > if (cf->can_dlc > 0) { > u32 data = be32_to_cpup((__be32 *)&cf->data[0]); > - flexcan_write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[0]); > + priv->write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[0]); > } > if (cf->can_dlc > 3) { > u32 data = be32_to_cpup((__be32 *)&cf->data[4]); > - flexcan_write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[1]); > + priv->write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[1]); > } > > can_put_echo_skb(skb, dev, 0); > > - flexcan_write(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id); > - flexcan_write(ctrl, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > + priv->write(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id); > + priv->write(ctrl, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > > /* Errata ERR005829 step8: > * Write twice INACTIVE(0x8) code to first MB. > */ > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); > > return NETDEV_TX_OK; > @@ -645,8 +654,8 @@ static void flexcan_read_fifo(const struct net_device *dev, > struct flexcan_mb __iomem *mb = ®s->cantxfg[0]; > u32 reg_ctrl, reg_id; > > - reg_ctrl = flexcan_read(&mb->can_ctrl); > - reg_id = flexcan_read(&mb->can_id); > + reg_ctrl = priv->read(&mb->can_ctrl); > + reg_id = priv->read(&mb->can_id); > if (reg_ctrl & FLEXCAN_MB_CNT_IDE) > cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG; > else > @@ -656,12 +665,12 @@ static void flexcan_read_fifo(const struct net_device *dev, > cf->can_id |= CAN_RTR_FLAG; > cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf); > > - *(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb->data[0])); > - *(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb->data[1])); > + *(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb->data[0])); > + *(__be32 *)(cf->data + 4) = cpu_to_be32(priv->read(&mb->data[1])); > > /* mark as read */ > - flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); > - flexcan_read(®s->timer); > + priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); > + priv->read(®s->timer); > } > > static int flexcan_read_frame(struct net_device *dev) > @@ -699,17 +708,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota) > * The error bits are cleared on read, > * use saved value from irq handler. > */ > - reg_esr = flexcan_read(®s->esr) | priv->reg_esr; > + reg_esr = priv->read(®s->esr) | priv->reg_esr; > > /* handle state changes */ > work_done += flexcan_poll_state(dev, reg_esr); > > /* handle RX-FIFO */ > - reg_iflag1 = flexcan_read(®s->iflag1); > + reg_iflag1 = priv->read(®s->iflag1); > while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE && > work_done < quota) { > work_done += flexcan_read_frame(dev); > - reg_iflag1 = flexcan_read(®s->iflag1); > + reg_iflag1 = priv->read(®s->iflag1); > } > > /* report bus errors */ > @@ -719,8 +728,8 @@ static int flexcan_poll(struct napi_struct *napi, int quota) > if (work_done < quota) { > napi_complete(napi); > /* enable IRQs */ > - flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > - flexcan_write(priv->reg_ctrl_default, ®s->ctrl); > + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > + priv->write(priv->reg_ctrl_default, ®s->ctrl); > } > > return work_done; > @@ -734,11 +743,11 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) > struct flexcan_regs __iomem *regs = priv->base; > u32 reg_iflag1, reg_esr; > > - reg_iflag1 = flexcan_read(®s->iflag1); > - reg_esr = flexcan_read(®s->esr); > + reg_iflag1 = priv->read(®s->iflag1); > + reg_esr = priv->read(®s->esr); > /* ACK all bus error and state change IRQ sources */ > if (reg_esr & FLEXCAN_ESR_ALL_INT) > - flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr); > + priv->write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr); > > /* > * schedule NAPI in case of: > @@ -754,16 +763,16 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) > * save them for later use. > */ > priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS; > - flexcan_write(FLEXCAN_IFLAG_DEFAULT & > + priv->write(FLEXCAN_IFLAG_DEFAULT & > ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->imask1); > - flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, > + priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, > ®s->ctrl); > napi_schedule(&priv->napi); > } > > /* FIFO overflow */ > if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) { > - flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, ®s->iflag1); > + priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, ®s->iflag1); > dev->stats.rx_over_errors++; > dev->stats.rx_errors++; > } > @@ -774,9 +783,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) > stats->tx_packets++; > can_led_event(dev, CAN_LED_EVENT_TX); > /* after sending a RTR frame mailbox is in RX mode */ > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > - flexcan_write((1 << FLEXCAN_TX_BUF_ID), ®s->iflag1); > + priv->write((1 << FLEXCAN_TX_BUF_ID), ®s->iflag1); > netif_wake_queue(dev); > } > > @@ -790,7 +799,7 @@ static void flexcan_set_bittiming(struct net_device *dev) > struct flexcan_regs __iomem *regs = priv->base; > u32 reg; > > - reg = flexcan_read(®s->ctrl); > + reg = priv->read(®s->ctrl); > reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) | > FLEXCAN_CTRL_RJW(0x3) | > FLEXCAN_CTRL_PSEG1(0x7) | > @@ -814,11 +823,11 @@ static void flexcan_set_bittiming(struct net_device *dev) > reg |= FLEXCAN_CTRL_SMP; > > netdev_info(dev, "writing ctrl=0x%08x\n", reg); > - flexcan_write(reg, ®s->ctrl); > + priv->write(reg, ®s->ctrl); > > /* print chip status */ > netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__, > - flexcan_read(®s->mcr), flexcan_read(®s->ctrl)); > + priv->read(®s->mcr), priv->read(®s->ctrl)); > } > > /* > @@ -858,14 +867,14 @@ static int flexcan_chip_start(struct net_device *dev) > * disable local echo > * > */ > - reg_mcr = flexcan_read(®s->mcr); > + reg_mcr = priv->read(®s->mcr); > reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff); > reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT | > FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN | > FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS | > FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID); > netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr); > - flexcan_write(reg_mcr, ®s->mcr); > + priv->write(reg_mcr, ®s->mcr); > > /* > * CTRL > @@ -879,7 +888,7 @@ static int flexcan_chip_start(struct net_device *dev) > * enable bus off interrupt > * (== FLEXCAN_CTRL_ERR_STATE) > */ > - reg_ctrl = flexcan_read(®s->ctrl); > + reg_ctrl = priv->read(®s->ctrl); > reg_ctrl &= ~FLEXCAN_CTRL_TSYN; > reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF | > FLEXCAN_CTRL_ERR_STATE; > @@ -897,29 +906,29 @@ static int flexcan_chip_start(struct net_device *dev) > /* save for later use */ > priv->reg_ctrl_default = reg_ctrl; > netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl); > - flexcan_write(reg_ctrl, ®s->ctrl); > + priv->write(reg_ctrl, ®s->ctrl); > > /* clear and invalidate all mailboxes first */ > for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->cantxfg); i++) { > - flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE, > + priv->write(FLEXCAN_MB_CODE_RX_INACTIVE, > ®s->cantxfg[i].can_ctrl); > } > > /* Errata ERR005829: mark first TX mailbox as INACTIVE */ > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); > > /* mark TX mailbox as INACTIVE */ > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > > /* acceptance mask/acceptance code (accept everything) */ > - flexcan_write(0x0, ®s->rxgmask); > - flexcan_write(0x0, ®s->rx14mask); > - flexcan_write(0x0, ®s->rx15mask); > + priv->write(0x0, ®s->rxgmask); > + priv->write(0x0, ®s->rx14mask); > + priv->write(0x0, ®s->rx15mask); > > if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) > - flexcan_write(0x0, ®s->rxfgmask); > + priv->write(0x0, ®s->rxfgmask); > > /* > * On Vybrid, disable memory error detection interrupts > @@ -934,16 +943,16 @@ static int flexcan_chip_start(struct net_device *dev) > * and Correction of Memory Errors" to write to > * MECR register > */ > - reg_crl2 = flexcan_read(®s->crl2); > + reg_crl2 = priv->read(®s->crl2); > reg_crl2 |= FLEXCAN_CRL2_ECRWRE; > - flexcan_write(reg_crl2, ®s->crl2); > + priv->write(reg_crl2, ®s->crl2); > > - reg_mecr = flexcan_read(®s->mecr); > + reg_mecr = priv->read(®s->mecr); > reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS; > - flexcan_write(reg_mecr, ®s->mecr); > + priv->write(reg_mecr, ®s->mecr); > reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ | FLEXCAN_MECR_HANCEI_MSK | > FLEXCAN_MECR_FANCEI_MSK); > - flexcan_write(reg_mecr, ®s->mecr); > + priv->write(reg_mecr, ®s->mecr); > } > > err = flexcan_transceiver_enable(priv); > @@ -958,11 +967,11 @@ static int flexcan_chip_start(struct net_device *dev) > priv->can.state = CAN_STATE_ERROR_ACTIVE; > > /* enable FIFO interrupts */ > - flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > > /* print chip status */ > netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__, > - flexcan_read(®s->mcr), flexcan_read(®s->ctrl)); > + priv->read(®s->mcr), priv->read(®s->ctrl)); > > return 0; > > @@ -989,8 +998,8 @@ static void flexcan_chip_stop(struct net_device *dev) > flexcan_chip_disable(priv); > > /* Disable all interrupts */ > - flexcan_write(0, ®s->imask1); > - flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, > + priv->write(0, ®s->imask1); > + priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, > ®s->ctrl); > > flexcan_transceiver_disable(priv); > @@ -1108,26 +1117,26 @@ static int register_flexcandev(struct net_device *dev) > err = flexcan_chip_disable(priv); > if (err) > goto out_disable_per; > - reg = flexcan_read(®s->ctrl); > + reg = priv->read(®s->ctrl); > reg |= FLEXCAN_CTRL_CLK_SRC; > - flexcan_write(reg, ®s->ctrl); > + priv->write(reg, ®s->ctrl); > > err = flexcan_chip_enable(priv); > if (err) > goto out_chip_disable; > > /* set freeze, halt and activate FIFO, restrict register access */ > - reg = flexcan_read(®s->mcr); > + reg = priv->read(®s->mcr); > reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | > FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV; > - flexcan_write(reg, ®s->mcr); > + priv->write(reg, ®s->mcr); > > /* > * Currently we only support newer versions of this core > * featuring a RX FIFO. Older cores found on some Coldfire > * derivates are not yet supported. > */ > - reg = flexcan_read(®s->mcr); > + reg = priv->read(®s->mcr); > if (!(reg & FLEXCAN_MCR_FEN)) { > netdev_err(dev, "Could not enable RX FIFO, unsupported core\n"); > err = -ENODEV; > @@ -1183,6 +1192,8 @@ static int flexcan_probe(struct platform_device *pdev) > void __iomem *base; > int err, irq; > u32 clock_freq = 0; > + /* Default case for most ARM based FSL SoC having BE FlexCAN IP */ > + bool core_is_little = true, module_is_little = false; > > reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver"); > if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER) > @@ -1237,6 +1248,25 @@ static int flexcan_probe(struct platform_device *pdev) > dev->flags |= IFF_ECHO; > > priv = netdev_priv(dev); > + > + if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) > + core_is_little = false; Why not just using IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) directly ? (anyways, cpu_is_le seems a more appropriate name) > + if (of_property_read_bool(dev->dev.of_node, "little-endian")) > + module_is_little = true; Why that misleading name, instead of, eg. "chip_is_le" ? Oh, and you'll expect explicitly set a new DTB attribute, if the chip is in LE - the default case. Didn't you just say, you dont wanna break anything ? Can you imagine that some people use DTB as a _board_ config (eg. in bootloader) instead of externalized kernel config ? ;-o > + if ((core_is_little && module_is_little) || > + (!core_is_little && !module_is_little)) { > + priv->read = flexcan_read_le; > + priv->write = flexcan_write_le; > + } > + > + if ((!core_is_little && module_is_little) || > + (core_is_little && !module_is_little)) { > + priv->read = flexcan_read_be; > + priv->write = flexcan_write_be; > + } At that point, the naming gets completely wrong - instead should be something like flexcan_read_swab / flexcan_write_swab. And the decision can easily be made on: IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) != module_is_little Putting these together, I'd rather: #1: add a new feature flag: #define FLEXCAN_HAS_BIG_ENDIAN BIT(4) #2: add a new dts device type for the new chip variant (eg. fsl,flexcan-qorliq) #2: pass the priv pointer to the *_read()/*_write() functions, so they can pick up the BE flag and decide whether to swab or not (based on cpu endianess) At that point, maybe we could even replace the struct flexcan_regs by a list of #define's. cu By the way: who had the genious idea of adding _hardware specific_ hacks into a widget library (qt) ? Perhaps the same guy, who decided that _unprivileged userland_ should tell ipu/vpu/gpu which _hardware_ addresses to use ? ;-o -- Enrico Weigelt, metux IT consult +49-151-27565287 MELAG Medizintechnik oHG Sitz Berlin Registergericht AG Charlottenburg HRA 21333 B Wichtiger Hinweis: Diese Nachricht kann vertrauliche oder nur für einen begrenzten Personenkreis bestimmte Informationen enthalten. Sie ist ausschließlich für denjenigen bestimmt, an den sie gerichtet worden ist. Wenn Sie nicht der Adressat dieser E-Mail sind, dürfen Sie diese nicht kopieren, weiterleiten, weitergeben oder sie ganz oder teilweise in irgendeiner Weise nutzen. Sollten Sie diese E-Mail irrtümlich erhalten haben, so benachrichtigen Sie bitte den Absender, indem Sie auf diese Nachricht antworten. Bitte löschen Sie in diesem Fall diese Nachricht und alle Anhänge, ohne eine Kopie zu behalten. Important Notice: This message may contain confidential or privileged information. It is intended only for the person it was addressed to. If you are not the intended recipient of this email you may not copy, forward, disclose or otherwise use it or any part of it in any form whatsoever. If you received this email in error please notify the sender by replying and delete this message and any attachments without retaining a copy. ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 4/5] can: flexcan: Remodel FlexCAN register r/w APIs for BE instances @ 2015-05-18 16:17 ` Enrico Weigelt, metux IT consult 0 siblings, 0 replies; 41+ messages in thread From: Enrico Weigelt, metux IT consult @ 2015-05-18 16:17 UTC (permalink / raw) To: linux-arm-kernel Am 14.05.2015 um 13:33 schrieb Bhupesh Sharma: > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > index deaa24e..b0222ae 100644 > --- a/drivers/net/can/flexcan.c > +++ b/drivers/net/can/flexcan.c > @@ -258,6 +258,10 @@ struct flexcan_priv { > struct flexcan_platform_data *pdata; > const struct flexcan_devtype_data *devtype_data; > struct regulator *reg_xceiver; > + > + /* Read and Write APIs */ > + u32 (*read)(void __iomem *addr); > + void (*write)(u32 val, void __iomem *addr); Does it *really* need to be a far call ? Why not just give the existing flexcan_read()/flexcan_write() inline's an additional bit to decide whether to swab or not ? > -#if defined(CONFIG_PPC) > -static inline u32 flexcan_read(void __iomem *addr) > +static inline u32 flexcan_read_le(void __iomem *addr) _static inline_ as a callback ? seriously ? > static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv) > { > @@ -356,14 +366,14 @@ static int flexcan_chip_enable(struct flexcan_priv *priv) > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > u32 reg; > > - reg = flexcan_read(®s->mcr); > + reg = priv->read(®s->mcr); > reg &= ~FLEXCAN_MCR_MDIS; > - flexcan_write(reg, ®s->mcr); > + priv->write(reg, ®s->mcr); > > - while (timeout-- && (flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > + while (timeout-- && (priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > udelay(10); > > - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK) > + if (priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK) > return -ETIMEDOUT; > > return 0; > @@ -375,14 +385,14 @@ static int flexcan_chip_disable(struct flexcan_priv *priv) > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > u32 reg; > > - reg = flexcan_read(®s->mcr); > + reg = priv->read(®s->mcr); > reg |= FLEXCAN_MCR_MDIS; > - flexcan_write(reg, ®s->mcr); > + priv->write(reg, ®s->mcr); > > - while (timeout-- && !(flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > + while (timeout-- && !(priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > udelay(10); > > - if (!(flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > + if (!(priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > return -ETIMEDOUT; > > return 0; > @@ -394,14 +404,14 @@ static int flexcan_chip_freeze(struct flexcan_priv *priv) > unsigned int timeout = 1000 * 1000 * 10 / priv->can.bittiming.bitrate; > u32 reg; > > - reg = flexcan_read(®s->mcr); > + reg = priv->read(®s->mcr); > reg |= FLEXCAN_MCR_HALT; > - flexcan_write(reg, ®s->mcr); > + priv->write(reg, ®s->mcr); > > - while (timeout-- && !(flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > + while (timeout-- && !(priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > udelay(100); > > - if (!(flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > + if (!(priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > return -ETIMEDOUT; > > return 0; > @@ -413,14 +423,14 @@ static int flexcan_chip_unfreeze(struct flexcan_priv *priv) > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > u32 reg; > > - reg = flexcan_read(®s->mcr); > + reg = priv->read(®s->mcr); > reg &= ~FLEXCAN_MCR_HALT; > - flexcan_write(reg, ®s->mcr); > + priv->write(reg, ®s->mcr); > > - while (timeout-- && (flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > + while (timeout-- && (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > udelay(10); > > - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK) > + if (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK) > return -ETIMEDOUT; > > return 0; > @@ -431,23 +441,22 @@ static int flexcan_chip_softreset(struct flexcan_priv *priv) > struct flexcan_regs __iomem *regs = priv->base; > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > > - flexcan_write(FLEXCAN_MCR_SOFTRST, ®s->mcr); > - while (timeout-- && (flexcan_read(®s->mcr) & FLEXCAN_MCR_SOFTRST)) > + priv->write(FLEXCAN_MCR_SOFTRST, ®s->mcr); > + while (timeout-- && (priv->read(®s->mcr) & FLEXCAN_MCR_SOFTRST)) > udelay(10); > > - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_SOFTRST) > + if (priv->read(®s->mcr) & FLEXCAN_MCR_SOFTRST) > return -ETIMEDOUT; > > return 0; > } > > - > static int __flexcan_get_berr_counter(const struct net_device *dev, > struct can_berr_counter *bec) > { > const struct flexcan_priv *priv = netdev_priv(dev); > struct flexcan_regs __iomem *regs = priv->base; > - u32 reg = flexcan_read(®s->ecr); > + u32 reg = priv->read(®s->ecr); > > bec->txerr = (reg >> 0) & 0xff; > bec->rxerr = (reg >> 8) & 0xff; > @@ -503,24 +512,24 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev) > > if (cf->can_dlc > 0) { > u32 data = be32_to_cpup((__be32 *)&cf->data[0]); > - flexcan_write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[0]); > + priv->write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[0]); > } > if (cf->can_dlc > 3) { > u32 data = be32_to_cpup((__be32 *)&cf->data[4]); > - flexcan_write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[1]); > + priv->write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[1]); > } > > can_put_echo_skb(skb, dev, 0); > > - flexcan_write(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id); > - flexcan_write(ctrl, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > + priv->write(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id); > + priv->write(ctrl, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > > /* Errata ERR005829 step8: > * Write twice INACTIVE(0x8) code to first MB. > */ > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); > > return NETDEV_TX_OK; > @@ -645,8 +654,8 @@ static void flexcan_read_fifo(const struct net_device *dev, > struct flexcan_mb __iomem *mb = ®s->cantxfg[0]; > u32 reg_ctrl, reg_id; > > - reg_ctrl = flexcan_read(&mb->can_ctrl); > - reg_id = flexcan_read(&mb->can_id); > + reg_ctrl = priv->read(&mb->can_ctrl); > + reg_id = priv->read(&mb->can_id); > if (reg_ctrl & FLEXCAN_MB_CNT_IDE) > cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG; > else > @@ -656,12 +665,12 @@ static void flexcan_read_fifo(const struct net_device *dev, > cf->can_id |= CAN_RTR_FLAG; > cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf); > > - *(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb->data[0])); > - *(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb->data[1])); > + *(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb->data[0])); > + *(__be32 *)(cf->data + 4) = cpu_to_be32(priv->read(&mb->data[1])); > > /* mark as read */ > - flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); > - flexcan_read(®s->timer); > + priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); > + priv->read(®s->timer); > } > > static int flexcan_read_frame(struct net_device *dev) > @@ -699,17 +708,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota) > * The error bits are cleared on read, > * use saved value from irq handler. > */ > - reg_esr = flexcan_read(®s->esr) | priv->reg_esr; > + reg_esr = priv->read(®s->esr) | priv->reg_esr; > > /* handle state changes */ > work_done += flexcan_poll_state(dev, reg_esr); > > /* handle RX-FIFO */ > - reg_iflag1 = flexcan_read(®s->iflag1); > + reg_iflag1 = priv->read(®s->iflag1); > while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE && > work_done < quota) { > work_done += flexcan_read_frame(dev); > - reg_iflag1 = flexcan_read(®s->iflag1); > + reg_iflag1 = priv->read(®s->iflag1); > } > > /* report bus errors */ > @@ -719,8 +728,8 @@ static int flexcan_poll(struct napi_struct *napi, int quota) > if (work_done < quota) { > napi_complete(napi); > /* enable IRQs */ > - flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > - flexcan_write(priv->reg_ctrl_default, ®s->ctrl); > + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > + priv->write(priv->reg_ctrl_default, ®s->ctrl); > } > > return work_done; > @@ -734,11 +743,11 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) > struct flexcan_regs __iomem *regs = priv->base; > u32 reg_iflag1, reg_esr; > > - reg_iflag1 = flexcan_read(®s->iflag1); > - reg_esr = flexcan_read(®s->esr); > + reg_iflag1 = priv->read(®s->iflag1); > + reg_esr = priv->read(®s->esr); > /* ACK all bus error and state change IRQ sources */ > if (reg_esr & FLEXCAN_ESR_ALL_INT) > - flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr); > + priv->write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr); > > /* > * schedule NAPI in case of: > @@ -754,16 +763,16 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) > * save them for later use. > */ > priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS; > - flexcan_write(FLEXCAN_IFLAG_DEFAULT & > + priv->write(FLEXCAN_IFLAG_DEFAULT & > ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->imask1); > - flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, > + priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, > ®s->ctrl); > napi_schedule(&priv->napi); > } > > /* FIFO overflow */ > if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) { > - flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, ®s->iflag1); > + priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, ®s->iflag1); > dev->stats.rx_over_errors++; > dev->stats.rx_errors++; > } > @@ -774,9 +783,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) > stats->tx_packets++; > can_led_event(dev, CAN_LED_EVENT_TX); > /* after sending a RTR frame mailbox is in RX mode */ > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > - flexcan_write((1 << FLEXCAN_TX_BUF_ID), ®s->iflag1); > + priv->write((1 << FLEXCAN_TX_BUF_ID), ®s->iflag1); > netif_wake_queue(dev); > } > > @@ -790,7 +799,7 @@ static void flexcan_set_bittiming(struct net_device *dev) > struct flexcan_regs __iomem *regs = priv->base; > u32 reg; > > - reg = flexcan_read(®s->ctrl); > + reg = priv->read(®s->ctrl); > reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) | > FLEXCAN_CTRL_RJW(0x3) | > FLEXCAN_CTRL_PSEG1(0x7) | > @@ -814,11 +823,11 @@ static void flexcan_set_bittiming(struct net_device *dev) > reg |= FLEXCAN_CTRL_SMP; > > netdev_info(dev, "writing ctrl=0x%08x\n", reg); > - flexcan_write(reg, ®s->ctrl); > + priv->write(reg, ®s->ctrl); > > /* print chip status */ > netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__, > - flexcan_read(®s->mcr), flexcan_read(®s->ctrl)); > + priv->read(®s->mcr), priv->read(®s->ctrl)); > } > > /* > @@ -858,14 +867,14 @@ static int flexcan_chip_start(struct net_device *dev) > * disable local echo > * > */ > - reg_mcr = flexcan_read(®s->mcr); > + reg_mcr = priv->read(®s->mcr); > reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff); > reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT | > FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN | > FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS | > FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID); > netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr); > - flexcan_write(reg_mcr, ®s->mcr); > + priv->write(reg_mcr, ®s->mcr); > > /* > * CTRL > @@ -879,7 +888,7 @@ static int flexcan_chip_start(struct net_device *dev) > * enable bus off interrupt > * (== FLEXCAN_CTRL_ERR_STATE) > */ > - reg_ctrl = flexcan_read(®s->ctrl); > + reg_ctrl = priv->read(®s->ctrl); > reg_ctrl &= ~FLEXCAN_CTRL_TSYN; > reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF | > FLEXCAN_CTRL_ERR_STATE; > @@ -897,29 +906,29 @@ static int flexcan_chip_start(struct net_device *dev) > /* save for later use */ > priv->reg_ctrl_default = reg_ctrl; > netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl); > - flexcan_write(reg_ctrl, ®s->ctrl); > + priv->write(reg_ctrl, ®s->ctrl); > > /* clear and invalidate all mailboxes first */ > for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->cantxfg); i++) { > - flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE, > + priv->write(FLEXCAN_MB_CODE_RX_INACTIVE, > ®s->cantxfg[i].can_ctrl); > } > > /* Errata ERR005829: mark first TX mailbox as INACTIVE */ > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); > > /* mark TX mailbox as INACTIVE */ > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > > /* acceptance mask/acceptance code (accept everything) */ > - flexcan_write(0x0, ®s->rxgmask); > - flexcan_write(0x0, ®s->rx14mask); > - flexcan_write(0x0, ®s->rx15mask); > + priv->write(0x0, ®s->rxgmask); > + priv->write(0x0, ®s->rx14mask); > + priv->write(0x0, ®s->rx15mask); > > if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) > - flexcan_write(0x0, ®s->rxfgmask); > + priv->write(0x0, ®s->rxfgmask); > > /* > * On Vybrid, disable memory error detection interrupts > @@ -934,16 +943,16 @@ static int flexcan_chip_start(struct net_device *dev) > * and Correction of Memory Errors" to write to > * MECR register > */ > - reg_crl2 = flexcan_read(®s->crl2); > + reg_crl2 = priv->read(®s->crl2); > reg_crl2 |= FLEXCAN_CRL2_ECRWRE; > - flexcan_write(reg_crl2, ®s->crl2); > + priv->write(reg_crl2, ®s->crl2); > > - reg_mecr = flexcan_read(®s->mecr); > + reg_mecr = priv->read(®s->mecr); > reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS; > - flexcan_write(reg_mecr, ®s->mecr); > + priv->write(reg_mecr, ®s->mecr); > reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ | FLEXCAN_MECR_HANCEI_MSK | > FLEXCAN_MECR_FANCEI_MSK); > - flexcan_write(reg_mecr, ®s->mecr); > + priv->write(reg_mecr, ®s->mecr); > } > > err = flexcan_transceiver_enable(priv); > @@ -958,11 +967,11 @@ static int flexcan_chip_start(struct net_device *dev) > priv->can.state = CAN_STATE_ERROR_ACTIVE; > > /* enable FIFO interrupts */ > - flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > > /* print chip status */ > netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__, > - flexcan_read(®s->mcr), flexcan_read(®s->ctrl)); > + priv->read(®s->mcr), priv->read(®s->ctrl)); > > return 0; > > @@ -989,8 +998,8 @@ static void flexcan_chip_stop(struct net_device *dev) > flexcan_chip_disable(priv); > > /* Disable all interrupts */ > - flexcan_write(0, ®s->imask1); > - flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, > + priv->write(0, ®s->imask1); > + priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, > ®s->ctrl); > > flexcan_transceiver_disable(priv); > @@ -1108,26 +1117,26 @@ static int register_flexcandev(struct net_device *dev) > err = flexcan_chip_disable(priv); > if (err) > goto out_disable_per; > - reg = flexcan_read(®s->ctrl); > + reg = priv->read(®s->ctrl); > reg |= FLEXCAN_CTRL_CLK_SRC; > - flexcan_write(reg, ®s->ctrl); > + priv->write(reg, ®s->ctrl); > > err = flexcan_chip_enable(priv); > if (err) > goto out_chip_disable; > > /* set freeze, halt and activate FIFO, restrict register access */ > - reg = flexcan_read(®s->mcr); > + reg = priv->read(®s->mcr); > reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | > FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV; > - flexcan_write(reg, ®s->mcr); > + priv->write(reg, ®s->mcr); > > /* > * Currently we only support newer versions of this core > * featuring a RX FIFO. Older cores found on some Coldfire > * derivates are not yet supported. > */ > - reg = flexcan_read(®s->mcr); > + reg = priv->read(®s->mcr); > if (!(reg & FLEXCAN_MCR_FEN)) { > netdev_err(dev, "Could not enable RX FIFO, unsupported core\n"); > err = -ENODEV; > @@ -1183,6 +1192,8 @@ static int flexcan_probe(struct platform_device *pdev) > void __iomem *base; > int err, irq; > u32 clock_freq = 0; > + /* Default case for most ARM based FSL SoC having BE FlexCAN IP */ > + bool core_is_little = true, module_is_little = false; > > reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver"); > if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER) > @@ -1237,6 +1248,25 @@ static int flexcan_probe(struct platform_device *pdev) > dev->flags |= IFF_ECHO; > > priv = netdev_priv(dev); > + > + if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) > + core_is_little = false; Why not just using IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) directly ? (anyways, cpu_is_le seems a more appropriate name) > + if (of_property_read_bool(dev->dev.of_node, "little-endian")) > + module_is_little = true; Why that misleading name, instead of, eg. "chip_is_le" ? Oh, and you'll expect explicitly set a new DTB attribute, if the chip is in LE - the default case. Didn't you just say, you dont wanna break anything ? Can you imagine that some people use DTB as a _board_ config (eg. in bootloader) instead of externalized kernel config ? ;-o > + if ((core_is_little && module_is_little) || > + (!core_is_little && !module_is_little)) { > + priv->read = flexcan_read_le; > + priv->write = flexcan_write_le; > + } > + > + if ((!core_is_little && module_is_little) || > + (core_is_little && !module_is_little)) { > + priv->read = flexcan_read_be; > + priv->write = flexcan_write_be; > + } At that point, the naming gets completely wrong - instead should be something like flexcan_read_swab / flexcan_write_swab. And the decision can easily be made on: IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) != module_is_little Putting these together, I'd rather: #1: add a new feature flag: #define FLEXCAN_HAS_BIG_ENDIAN BIT(4) #2: add a new dts device type for the new chip variant (eg. fsl,flexcan-qorliq) #2: pass the priv pointer to the *_read()/*_write() functions, so they can pick up the BE flag and decide whether to swab or not (based on cpu endianess) At that point, maybe we could even replace the struct flexcan_regs by a list of #define's. cu By the way: who had the genious idea of adding _hardware specific_ hacks into a widget library (qt) ? Perhaps the same guy, who decided that _unprivileged userland_ should tell ipu/vpu/gpu which _hardware_ addresses to use ? ;-o -- Enrico Weigelt, metux IT consult +49-151-27565287 MELAG Medizintechnik oHG Sitz Berlin Registergericht AG Charlottenburg HRA 21333 B Wichtiger Hinweis: Diese Nachricht kann vertrauliche oder nur f?r einen begrenzten Personenkreis bestimmte Informationen enthalten. Sie ist ausschlie?lich f?r denjenigen bestimmt, an den sie gerichtet worden ist. Wenn Sie nicht der Adressat dieser E-Mail sind, d?rfen Sie diese nicht kopieren, weiterleiten, weitergeben oder sie ganz oder teilweise in irgendeiner Weise nutzen. Sollten Sie diese E-Mail irrt?mlich erhalten haben, so benachrichtigen Sie bitte den Absender, indem Sie auf diese Nachricht antworten. Bitte l?schen Sie in diesem Fall diese Nachricht und alle Anh?nge, ohne eine Kopie zu behalten. Important Notice: This message may contain confidential or privileged information. It is intended only for the person it was addressed to. If you are not the intended recipient of this email you may not copy, forward, disclose or otherwise use it or any part of it in any form whatsoever. If you received this email in error please notify the sender by replying and delete this message and any attachments without retaining a copy. ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v2 4/5] can: flexcan: Remodel FlexCAN register r/w APIs for BE instances 2015-05-18 16:17 ` Enrico Weigelt, metux IT consult @ 2015-05-18 16:37 ` Sharma Bhupesh -1 siblings, 0 replies; 41+ messages in thread From: Sharma Bhupesh @ 2015-05-18 16:37 UTC (permalink / raw) To: Enrico Weigelt, metux IT consult, arnd, linux-can, mkl Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar > -----Original Message----- > From: Enrico Weigelt, metux IT consult [mailto:weigelt@melag.de] > Am 14.05.2015 um 13:33 schrieb Bhupesh Sharma: > > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > > index deaa24e..b0222ae 100644 > > --- a/drivers/net/can/flexcan.c > > +++ b/drivers/net/can/flexcan.c > > @@ -258,6 +258,10 @@ struct flexcan_priv { > > struct flexcan_platform_data *pdata; > > const struct flexcan_devtype_data *devtype_data; > > struct regulator *reg_xceiver; > > + > > + /* Read and Write APIs */ > > + u32 (*read)(void __iomem *addr); > > + void (*write)(u32 val, void __iomem *addr); > > Does it *really* need to be a far call ? > Why not just give the existing flexcan_read()/flexcan_write() inline's an > additional bit to decide whether to swab or not ? > > > -#if defined(CONFIG_PPC) > > -static inline u32 flexcan_read(void __iomem *addr) > > +static inline u32 flexcan_read_le(void __iomem *addr) > > _static inline_ as a callback ? seriously ? > > > static inline int flexcan_transceiver_enable(const struct > flexcan_priv *priv) > > { > > @@ -356,14 +366,14 @@ static int flexcan_chip_enable(struct > flexcan_priv *priv) > > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > > u32 reg; > > > > - reg = flexcan_read(®s->mcr); > > + reg = priv->read(®s->mcr); > > reg &= ~FLEXCAN_MCR_MDIS; > > - flexcan_write(reg, ®s->mcr); > > + priv->write(reg, ®s->mcr); > > > > - while (timeout-- && (flexcan_read(®s->mcr) & > FLEXCAN_MCR_LPM_ACK)) > > + while (timeout-- && (priv->read(®s->mcr) & > > + FLEXCAN_MCR_LPM_ACK)) > > udelay(10); > > > > - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK) > > + if (priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK) > > return -ETIMEDOUT; > > > > return 0; > > @@ -375,14 +385,14 @@ static int flexcan_chip_disable(struct > flexcan_priv *priv) > > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > > u32 reg; > > > > - reg = flexcan_read(®s->mcr); > > + reg = priv->read(®s->mcr); > > reg |= FLEXCAN_MCR_MDIS; > > - flexcan_write(reg, ®s->mcr); > > + priv->write(reg, ®s->mcr); > > > > - while (timeout-- && !(flexcan_read(®s->mcr) & > FLEXCAN_MCR_LPM_ACK)) > > + while (timeout-- && !(priv->read(®s->mcr) & > > + FLEXCAN_MCR_LPM_ACK)) > > udelay(10); > > > > - if (!(flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > > + if (!(priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > > return -ETIMEDOUT; > > > > return 0; > > @@ -394,14 +404,14 @@ static int flexcan_chip_freeze(struct > flexcan_priv *priv) > > unsigned int timeout = 1000 * 1000 * 10 / priv- > >can.bittiming.bitrate; > > u32 reg; > > > > - reg = flexcan_read(®s->mcr); > > + reg = priv->read(®s->mcr); > > reg |= FLEXCAN_MCR_HALT; > > - flexcan_write(reg, ®s->mcr); > > + priv->write(reg, ®s->mcr); > > > > - while (timeout-- && !(flexcan_read(®s->mcr) & > FLEXCAN_MCR_FRZ_ACK)) > > + while (timeout-- && !(priv->read(®s->mcr) & > > + FLEXCAN_MCR_FRZ_ACK)) > > udelay(100); > > > > - if (!(flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > > + if (!(priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > > return -ETIMEDOUT; > > > > return 0; > > @@ -413,14 +423,14 @@ static int flexcan_chip_unfreeze(struct > flexcan_priv *priv) > > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > > u32 reg; > > > > - reg = flexcan_read(®s->mcr); > > + reg = priv->read(®s->mcr); > > reg &= ~FLEXCAN_MCR_HALT; > > - flexcan_write(reg, ®s->mcr); > > + priv->write(reg, ®s->mcr); > > > > - while (timeout-- && (flexcan_read(®s->mcr) & > FLEXCAN_MCR_FRZ_ACK)) > > + while (timeout-- && (priv->read(®s->mcr) & > > + FLEXCAN_MCR_FRZ_ACK)) > > udelay(10); > > > > - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK) > > + if (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK) > > return -ETIMEDOUT; > > > > return 0; > > @@ -431,23 +441,22 @@ static int flexcan_chip_softreset(struct > flexcan_priv *priv) > > struct flexcan_regs __iomem *regs = priv->base; > > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > > > > - flexcan_write(FLEXCAN_MCR_SOFTRST, ®s->mcr); > > - while (timeout-- && (flexcan_read(®s->mcr) & > FLEXCAN_MCR_SOFTRST)) > > + priv->write(FLEXCAN_MCR_SOFTRST, ®s->mcr); > > + while (timeout-- && (priv->read(®s->mcr) & > > + FLEXCAN_MCR_SOFTRST)) > > udelay(10); > > > > - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_SOFTRST) > > + if (priv->read(®s->mcr) & FLEXCAN_MCR_SOFTRST) > > return -ETIMEDOUT; > > > > return 0; > > } > > > > - > > static int __flexcan_get_berr_counter(const struct net_device *dev, > > struct can_berr_counter *bec) > > { > > const struct flexcan_priv *priv = netdev_priv(dev); > > struct flexcan_regs __iomem *regs = priv->base; > > - u32 reg = flexcan_read(®s->ecr); > > + u32 reg = priv->read(®s->ecr); > > > > bec->txerr = (reg >> 0) & 0xff; > > bec->rxerr = (reg >> 8) & 0xff; > > @@ -503,24 +512,24 @@ static int flexcan_start_xmit(struct sk_buff > > *skb, struct net_device *dev) > > > > if (cf->can_dlc > 0) { > > u32 data = be32_to_cpup((__be32 *)&cf->data[0]); > > - flexcan_write(data, ®s- > >cantxfg[FLEXCAN_TX_BUF_ID].data[0]); > > + priv->write(data, > > + ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[0]); > > } > > if (cf->can_dlc > 3) { > > u32 data = be32_to_cpup((__be32 *)&cf->data[4]); > > - flexcan_write(data, ®s- > >cantxfg[FLEXCAN_TX_BUF_ID].data[1]); > > + priv->write(data, > > + ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[1]); > > } > > > > can_put_echo_skb(skb, dev, 0); > > > > - flexcan_write(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id); > > - flexcan_write(ctrl, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > > + priv->write(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id); > > + priv->write(ctrl, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > > > > /* Errata ERR005829 step8: > > * Write twice INACTIVE(0x8) code to first MB. > > */ > > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > > ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); > > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > > ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); > > > > return NETDEV_TX_OK; > > @@ -645,8 +654,8 @@ static void flexcan_read_fifo(const struct > net_device *dev, > > struct flexcan_mb __iomem *mb = ®s->cantxfg[0]; > > u32 reg_ctrl, reg_id; > > > > - reg_ctrl = flexcan_read(&mb->can_ctrl); > > - reg_id = flexcan_read(&mb->can_id); > > + reg_ctrl = priv->read(&mb->can_ctrl); > > + reg_id = priv->read(&mb->can_id); > > if (reg_ctrl & FLEXCAN_MB_CNT_IDE) > > cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | > CAN_EFF_FLAG; > > else > > @@ -656,12 +665,12 @@ static void flexcan_read_fifo(const struct > net_device *dev, > > cf->can_id |= CAN_RTR_FLAG; > > cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf); > > > > - *(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb- > >data[0])); > > - *(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb- > >data[1])); > > + *(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb- > >data[0])); > > + *(__be32 *)(cf->data + 4) = > > + cpu_to_be32(priv->read(&mb->data[1])); > > > > /* mark as read */ > > - flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); > > - flexcan_read(®s->timer); > > + priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); > > + priv->read(®s->timer); > > } > > > > static int flexcan_read_frame(struct net_device *dev) @@ -699,17 > > +708,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota) > > * The error bits are cleared on read, > > * use saved value from irq handler. > > */ > > - reg_esr = flexcan_read(®s->esr) | priv->reg_esr; > > + reg_esr = priv->read(®s->esr) | priv->reg_esr; > > > > /* handle state changes */ > > work_done += flexcan_poll_state(dev, reg_esr); > > > > /* handle RX-FIFO */ > > - reg_iflag1 = flexcan_read(®s->iflag1); > > + reg_iflag1 = priv->read(®s->iflag1); > > while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE && > > work_done < quota) { > > work_done += flexcan_read_frame(dev); > > - reg_iflag1 = flexcan_read(®s->iflag1); > > + reg_iflag1 = priv->read(®s->iflag1); > > } > > > > /* report bus errors */ > > @@ -719,8 +728,8 @@ static int flexcan_poll(struct napi_struct *napi, > int quota) > > if (work_done < quota) { > > napi_complete(napi); > > /* enable IRQs */ > > - flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > > - flexcan_write(priv->reg_ctrl_default, ®s->ctrl); > > + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > > + priv->write(priv->reg_ctrl_default, ®s->ctrl); > > } > > > > return work_done; > > @@ -734,11 +743,11 @@ static irqreturn_t flexcan_irq(int irq, void > *dev_id) > > struct flexcan_regs __iomem *regs = priv->base; > > u32 reg_iflag1, reg_esr; > > > > - reg_iflag1 = flexcan_read(®s->iflag1); > > - reg_esr = flexcan_read(®s->esr); > > + reg_iflag1 = priv->read(®s->iflag1); > > + reg_esr = priv->read(®s->esr); > > /* ACK all bus error and state change IRQ sources */ > > if (reg_esr & FLEXCAN_ESR_ALL_INT) > > - flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr); > > + priv->write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr); > > > > /* > > * schedule NAPI in case of: > > @@ -754,16 +763,16 @@ static irqreturn_t flexcan_irq(int irq, void > *dev_id) > > * save them for later use. > > */ > > priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS; > > - flexcan_write(FLEXCAN_IFLAG_DEFAULT & > > + priv->write(FLEXCAN_IFLAG_DEFAULT & > > ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->imask1); > > - flexcan_write(priv->reg_ctrl_default & > ~FLEXCAN_CTRL_ERR_ALL, > > + priv->write(priv->reg_ctrl_default & > > + ~FLEXCAN_CTRL_ERR_ALL, > > ®s->ctrl); > > napi_schedule(&priv->napi); > > } > > > > /* FIFO overflow */ > > if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) { > > - flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, ®s- > >iflag1); > > + priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, > > + ®s->iflag1); > > dev->stats.rx_over_errors++; > > dev->stats.rx_errors++; > > } > > @@ -774,9 +783,9 @@ static irqreturn_t flexcan_irq(int irq, void > *dev_id) > > stats->tx_packets++; > > can_led_event(dev, CAN_LED_EVENT_TX); > > /* after sending a RTR frame mailbox is in RX mode */ > > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > > ®s- > >cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > > - flexcan_write((1 << FLEXCAN_TX_BUF_ID), ®s->iflag1); > > + priv->write((1 << FLEXCAN_TX_BUF_ID), ®s->iflag1); > > netif_wake_queue(dev); > > } > > > > @@ -790,7 +799,7 @@ static void flexcan_set_bittiming(struct net_device > *dev) > > struct flexcan_regs __iomem *regs = priv->base; > > u32 reg; > > > > - reg = flexcan_read(®s->ctrl); > > + reg = priv->read(®s->ctrl); > > reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) | > > FLEXCAN_CTRL_RJW(0x3) | > > FLEXCAN_CTRL_PSEG1(0x7) | @@ -814,11 +823,11 @@ static > > void flexcan_set_bittiming(struct net_device *dev) > > reg |= FLEXCAN_CTRL_SMP; > > > > netdev_info(dev, "writing ctrl=0x%08x\n", reg); > > - flexcan_write(reg, ®s->ctrl); > > + priv->write(reg, ®s->ctrl); > > > > /* print chip status */ > > netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__, > > - flexcan_read(®s->mcr), flexcan_read(®s->ctrl)); > > + priv->read(®s->mcr), priv->read(®s->ctrl)); > > } > > > > /* > > @@ -858,14 +867,14 @@ static int flexcan_chip_start(struct net_device > *dev) > > * disable local echo > > * > > */ > > - reg_mcr = flexcan_read(®s->mcr); > > + reg_mcr = priv->read(®s->mcr); > > reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff); > > reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT | > > FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN | > > FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS | > > FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID); > > netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr); > > - flexcan_write(reg_mcr, ®s->mcr); > > + priv->write(reg_mcr, ®s->mcr); > > > > /* > > * CTRL > > @@ -879,7 +888,7 @@ static int flexcan_chip_start(struct net_device > *dev) > > * enable bus off interrupt > > * (== FLEXCAN_CTRL_ERR_STATE) > > */ > > - reg_ctrl = flexcan_read(®s->ctrl); > > + reg_ctrl = priv->read(®s->ctrl); > > reg_ctrl &= ~FLEXCAN_CTRL_TSYN; > > reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF | > > FLEXCAN_CTRL_ERR_STATE; > > @@ -897,29 +906,29 @@ static int flexcan_chip_start(struct net_device > *dev) > > /* save for later use */ > > priv->reg_ctrl_default = reg_ctrl; > > netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl); > > - flexcan_write(reg_ctrl, ®s->ctrl); > > + priv->write(reg_ctrl, ®s->ctrl); > > > > /* clear and invalidate all mailboxes first */ > > for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->cantxfg); i++) { > > - flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE, > > + priv->write(FLEXCAN_MB_CODE_RX_INACTIVE, > > ®s->cantxfg[i].can_ctrl); > > } > > > > /* Errata ERR005829: mark first TX mailbox as INACTIVE */ > > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > > ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); > > > > /* mark TX mailbox as INACTIVE */ > > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > > ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > > > > /* acceptance mask/acceptance code (accept everything) */ > > - flexcan_write(0x0, ®s->rxgmask); > > - flexcan_write(0x0, ®s->rx14mask); > > - flexcan_write(0x0, ®s->rx15mask); > > + priv->write(0x0, ®s->rxgmask); > > + priv->write(0x0, ®s->rx14mask); > > + priv->write(0x0, ®s->rx15mask); > > > > if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) > > - flexcan_write(0x0, ®s->rxfgmask); > > + priv->write(0x0, ®s->rxfgmask); > > > > /* > > * On Vybrid, disable memory error detection interrupts @@ > > -934,16 +943,16 @@ static int flexcan_chip_start(struct net_device > *dev) > > * and Correction of Memory Errors" to write to > > * MECR register > > */ > > - reg_crl2 = flexcan_read(®s->crl2); > > + reg_crl2 = priv->read(®s->crl2); > > reg_crl2 |= FLEXCAN_CRL2_ECRWRE; > > - flexcan_write(reg_crl2, ®s->crl2); > > + priv->write(reg_crl2, ®s->crl2); > > > > - reg_mecr = flexcan_read(®s->mecr); > > + reg_mecr = priv->read(®s->mecr); > > reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS; > > - flexcan_write(reg_mecr, ®s->mecr); > > + priv->write(reg_mecr, ®s->mecr); > > reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ | > FLEXCAN_MECR_HANCEI_MSK | > > FLEXCAN_MECR_FANCEI_MSK); > > - flexcan_write(reg_mecr, ®s->mecr); > > + priv->write(reg_mecr, ®s->mecr); > > } > > > > err = flexcan_transceiver_enable(priv); @@ -958,11 +967,11 @@ > > static int flexcan_chip_start(struct net_device *dev) > > priv->can.state = CAN_STATE_ERROR_ACTIVE; > > > > /* enable FIFO interrupts */ > > - flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > > + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > > > > /* print chip status */ > > netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__, > > - flexcan_read(®s->mcr), flexcan_read(®s->ctrl)); > > + priv->read(®s->mcr), priv->read(®s->ctrl)); > > > > return 0; > > > > @@ -989,8 +998,8 @@ static void flexcan_chip_stop(struct net_device > *dev) > > flexcan_chip_disable(priv); > > > > /* Disable all interrupts */ > > - flexcan_write(0, ®s->imask1); > > - flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, > > + priv->write(0, ®s->imask1); > > + priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, > > ®s->ctrl); > > > > flexcan_transceiver_disable(priv); > > @@ -1108,26 +1117,26 @@ static int register_flexcandev(struct > net_device *dev) > > err = flexcan_chip_disable(priv); > > if (err) > > goto out_disable_per; > > - reg = flexcan_read(®s->ctrl); > > + reg = priv->read(®s->ctrl); > > reg |= FLEXCAN_CTRL_CLK_SRC; > > - flexcan_write(reg, ®s->ctrl); > > + priv->write(reg, ®s->ctrl); > > > > err = flexcan_chip_enable(priv); > > if (err) > > goto out_chip_disable; > > > > /* set freeze, halt and activate FIFO, restrict register access > */ > > - reg = flexcan_read(®s->mcr); > > + reg = priv->read(®s->mcr); > > reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | > > FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV; > > - flexcan_write(reg, ®s->mcr); > > + priv->write(reg, ®s->mcr); > > > > /* > > * Currently we only support newer versions of this core > > * featuring a RX FIFO. Older cores found on some Coldfire > > * derivates are not yet supported. > > */ > > - reg = flexcan_read(®s->mcr); > > + reg = priv->read(®s->mcr); > > if (!(reg & FLEXCAN_MCR_FEN)) { > > netdev_err(dev, "Could not enable RX FIFO, unsupported > core\n"); > > err = -ENODEV; > > @@ -1183,6 +1192,8 @@ static int flexcan_probe(struct platform_device > *pdev) > > void __iomem *base; > > int err, irq; > > u32 clock_freq = 0; > > + /* Default case for most ARM based FSL SoC having BE FlexCAN IP > */ > > + bool core_is_little = true, module_is_little = false; > > > > reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver"); > > if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER) @@ -1237,6 +1248,25 > > @@ static int flexcan_probe(struct platform_device *pdev) > > dev->flags |= IFF_ECHO; > > > > priv = netdev_priv(dev); > > + > > + if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) > > + core_is_little = false; > > Why not just using IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) directly ? > (anyways, cpu_is_le seems a more appropriate name) > > > + if (of_property_read_bool(dev->dev.of_node, "little-endian")) > > + module_is_little = true; > > Why that misleading name, instead of, eg. "chip_is_le" ? > > Oh, and you'll expect explicitly set a new DTB attribute, if the chip is > in LE - the default case. > > Didn't you just say, you dont wanna break anything ? > > Can you imagine that some people use DTB as a _board_ config (eg. in > bootloader) instead of externalized kernel config ? ;-o > > > + if ((core_is_little && module_is_little) || > > + (!core_is_little && !module_is_little)) { > > + priv->read = flexcan_read_le; > > + priv->write = flexcan_write_le; > > + } > > + > > + if ((!core_is_little && module_is_little) || > > + (core_is_little && !module_is_little)) { > > + priv->read = flexcan_read_be; > > + priv->write = flexcan_write_be; > > + } > > At that point, the naming gets completely wrong - instead should be > something like flexcan_read_swab / flexcan_write_swab. > And the decision can easily be made on: > > IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) != module_is_little > > > Putting these together, I'd rather: > > #1: add a new feature flag: > #define FLEXCAN_HAS_BIG_ENDIAN BIT(4) > > #2: add a new dts device type for the new chip variant > (eg. fsl,flexcan-qorliq) > > #2: pass the priv pointer to the *_read()/*_write() functions, > so they can pick up the BE flag and decide whether to > swab or not (based on cpu endianess) > You seem to have missed the endian property discussions that happened regarding DTS Several months back. Have a look at them: [1] & [2]. This patch is compliant to various DTS that already have such a property. And BTW, I have already mentioned this during the v1 review of this patchset: We are dealing with 4 possible cases here: 1. Core is LE (ARM) <-> FlexCAN IP is BE = FSL LS1021A SoC 2. Core is BE (PPC) <-> FlexCAN IP is BE = FSL P1010 SoC 3. Core is BE (PPC) <-> FlexCAN IP is LE = No SoC with this configuration yet 4. Core is LE (PPC) <-> FlexCAN IP is LE = FSL I.MX series So, no this _simply_ won't work the way you described for the above cases. I remember testing this patchset also on a P1010, so I don't find any legacy stuff breaking. [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/254533.html [2] http://lxr.free-electrons.com/source/arch/arm/boot/dts/ls1021a.dtsi#L127 Regards, Bhupesh ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 4/5] can: flexcan: Remodel FlexCAN register r/w APIs for BE instances @ 2015-05-18 16:37 ` Sharma Bhupesh 0 siblings, 0 replies; 41+ messages in thread From: Sharma Bhupesh @ 2015-05-18 16:37 UTC (permalink / raw) To: linux-arm-kernel > -----Original Message----- > From: Enrico Weigelt, metux IT consult [mailto:weigelt at melag.de] > Am 14.05.2015 um 13:33 schrieb Bhupesh Sharma: > > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > > index deaa24e..b0222ae 100644 > > --- a/drivers/net/can/flexcan.c > > +++ b/drivers/net/can/flexcan.c > > @@ -258,6 +258,10 @@ struct flexcan_priv { > > struct flexcan_platform_data *pdata; > > const struct flexcan_devtype_data *devtype_data; > > struct regulator *reg_xceiver; > > + > > + /* Read and Write APIs */ > > + u32 (*read)(void __iomem *addr); > > + void (*write)(u32 val, void __iomem *addr); > > Does it *really* need to be a far call ? > Why not just give the existing flexcan_read()/flexcan_write() inline's an > additional bit to decide whether to swab or not ? > > > -#if defined(CONFIG_PPC) > > -static inline u32 flexcan_read(void __iomem *addr) > > +static inline u32 flexcan_read_le(void __iomem *addr) > > _static inline_ as a callback ? seriously ? > > > static inline int flexcan_transceiver_enable(const struct > flexcan_priv *priv) > > { > > @@ -356,14 +366,14 @@ static int flexcan_chip_enable(struct > flexcan_priv *priv) > > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > > u32 reg; > > > > - reg = flexcan_read(®s->mcr); > > + reg = priv->read(®s->mcr); > > reg &= ~FLEXCAN_MCR_MDIS; > > - flexcan_write(reg, ®s->mcr); > > + priv->write(reg, ®s->mcr); > > > > - while (timeout-- && (flexcan_read(®s->mcr) & > FLEXCAN_MCR_LPM_ACK)) > > + while (timeout-- && (priv->read(®s->mcr) & > > + FLEXCAN_MCR_LPM_ACK)) > > udelay(10); > > > > - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK) > > + if (priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK) > > return -ETIMEDOUT; > > > > return 0; > > @@ -375,14 +385,14 @@ static int flexcan_chip_disable(struct > flexcan_priv *priv) > > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > > u32 reg; > > > > - reg = flexcan_read(®s->mcr); > > + reg = priv->read(®s->mcr); > > reg |= FLEXCAN_MCR_MDIS; > > - flexcan_write(reg, ®s->mcr); > > + priv->write(reg, ®s->mcr); > > > > - while (timeout-- && !(flexcan_read(®s->mcr) & > FLEXCAN_MCR_LPM_ACK)) > > + while (timeout-- && !(priv->read(®s->mcr) & > > + FLEXCAN_MCR_LPM_ACK)) > > udelay(10); > > > > - if (!(flexcan_read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > > + if (!(priv->read(®s->mcr) & FLEXCAN_MCR_LPM_ACK)) > > return -ETIMEDOUT; > > > > return 0; > > @@ -394,14 +404,14 @@ static int flexcan_chip_freeze(struct > flexcan_priv *priv) > > unsigned int timeout = 1000 * 1000 * 10 / priv- > >can.bittiming.bitrate; > > u32 reg; > > > > - reg = flexcan_read(®s->mcr); > > + reg = priv->read(®s->mcr); > > reg |= FLEXCAN_MCR_HALT; > > - flexcan_write(reg, ®s->mcr); > > + priv->write(reg, ®s->mcr); > > > > - while (timeout-- && !(flexcan_read(®s->mcr) & > FLEXCAN_MCR_FRZ_ACK)) > > + while (timeout-- && !(priv->read(®s->mcr) & > > + FLEXCAN_MCR_FRZ_ACK)) > > udelay(100); > > > > - if (!(flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > > + if (!(priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK)) > > return -ETIMEDOUT; > > > > return 0; > > @@ -413,14 +423,14 @@ static int flexcan_chip_unfreeze(struct > flexcan_priv *priv) > > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > > u32 reg; > > > > - reg = flexcan_read(®s->mcr); > > + reg = priv->read(®s->mcr); > > reg &= ~FLEXCAN_MCR_HALT; > > - flexcan_write(reg, ®s->mcr); > > + priv->write(reg, ®s->mcr); > > > > - while (timeout-- && (flexcan_read(®s->mcr) & > FLEXCAN_MCR_FRZ_ACK)) > > + while (timeout-- && (priv->read(®s->mcr) & > > + FLEXCAN_MCR_FRZ_ACK)) > > udelay(10); > > > > - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK) > > + if (priv->read(®s->mcr) & FLEXCAN_MCR_FRZ_ACK) > > return -ETIMEDOUT; > > > > return 0; > > @@ -431,23 +441,22 @@ static int flexcan_chip_softreset(struct > flexcan_priv *priv) > > struct flexcan_regs __iomem *regs = priv->base; > > unsigned int timeout = FLEXCAN_TIMEOUT_US / 10; > > > > - flexcan_write(FLEXCAN_MCR_SOFTRST, ®s->mcr); > > - while (timeout-- && (flexcan_read(®s->mcr) & > FLEXCAN_MCR_SOFTRST)) > > + priv->write(FLEXCAN_MCR_SOFTRST, ®s->mcr); > > + while (timeout-- && (priv->read(®s->mcr) & > > + FLEXCAN_MCR_SOFTRST)) > > udelay(10); > > > > - if (flexcan_read(®s->mcr) & FLEXCAN_MCR_SOFTRST) > > + if (priv->read(®s->mcr) & FLEXCAN_MCR_SOFTRST) > > return -ETIMEDOUT; > > > > return 0; > > } > > > > - > > static int __flexcan_get_berr_counter(const struct net_device *dev, > > struct can_berr_counter *bec) > > { > > const struct flexcan_priv *priv = netdev_priv(dev); > > struct flexcan_regs __iomem *regs = priv->base; > > - u32 reg = flexcan_read(®s->ecr); > > + u32 reg = priv->read(®s->ecr); > > > > bec->txerr = (reg >> 0) & 0xff; > > bec->rxerr = (reg >> 8) & 0xff; > > @@ -503,24 +512,24 @@ static int flexcan_start_xmit(struct sk_buff > > *skb, struct net_device *dev) > > > > if (cf->can_dlc > 0) { > > u32 data = be32_to_cpup((__be32 *)&cf->data[0]); > > - flexcan_write(data, ®s- > >cantxfg[FLEXCAN_TX_BUF_ID].data[0]); > > + priv->write(data, > > + ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[0]); > > } > > if (cf->can_dlc > 3) { > > u32 data = be32_to_cpup((__be32 *)&cf->data[4]); > > - flexcan_write(data, ®s- > >cantxfg[FLEXCAN_TX_BUF_ID].data[1]); > > + priv->write(data, > > + ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[1]); > > } > > > > can_put_echo_skb(skb, dev, 0); > > > > - flexcan_write(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id); > > - flexcan_write(ctrl, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > > + priv->write(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id); > > + priv->write(ctrl, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > > > > /* Errata ERR005829 step8: > > * Write twice INACTIVE(0x8) code to first MB. > > */ > > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > > ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); > > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > > ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); > > > > return NETDEV_TX_OK; > > @@ -645,8 +654,8 @@ static void flexcan_read_fifo(const struct > net_device *dev, > > struct flexcan_mb __iomem *mb = ®s->cantxfg[0]; > > u32 reg_ctrl, reg_id; > > > > - reg_ctrl = flexcan_read(&mb->can_ctrl); > > - reg_id = flexcan_read(&mb->can_id); > > + reg_ctrl = priv->read(&mb->can_ctrl); > > + reg_id = priv->read(&mb->can_id); > > if (reg_ctrl & FLEXCAN_MB_CNT_IDE) > > cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | > CAN_EFF_FLAG; > > else > > @@ -656,12 +665,12 @@ static void flexcan_read_fifo(const struct > net_device *dev, > > cf->can_id |= CAN_RTR_FLAG; > > cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf); > > > > - *(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb- > >data[0])); > > - *(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb- > >data[1])); > > + *(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb- > >data[0])); > > + *(__be32 *)(cf->data + 4) = > > + cpu_to_be32(priv->read(&mb->data[1])); > > > > /* mark as read */ > > - flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); > > - flexcan_read(®s->timer); > > + priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); > > + priv->read(®s->timer); > > } > > > > static int flexcan_read_frame(struct net_device *dev) @@ -699,17 > > +708,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota) > > * The error bits are cleared on read, > > * use saved value from irq handler. > > */ > > - reg_esr = flexcan_read(®s->esr) | priv->reg_esr; > > + reg_esr = priv->read(®s->esr) | priv->reg_esr; > > > > /* handle state changes */ > > work_done += flexcan_poll_state(dev, reg_esr); > > > > /* handle RX-FIFO */ > > - reg_iflag1 = flexcan_read(®s->iflag1); > > + reg_iflag1 = priv->read(®s->iflag1); > > while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE && > > work_done < quota) { > > work_done += flexcan_read_frame(dev); > > - reg_iflag1 = flexcan_read(®s->iflag1); > > + reg_iflag1 = priv->read(®s->iflag1); > > } > > > > /* report bus errors */ > > @@ -719,8 +728,8 @@ static int flexcan_poll(struct napi_struct *napi, > int quota) > > if (work_done < quota) { > > napi_complete(napi); > > /* enable IRQs */ > > - flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > > - flexcan_write(priv->reg_ctrl_default, ®s->ctrl); > > + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > > + priv->write(priv->reg_ctrl_default, ®s->ctrl); > > } > > > > return work_done; > > @@ -734,11 +743,11 @@ static irqreturn_t flexcan_irq(int irq, void > *dev_id) > > struct flexcan_regs __iomem *regs = priv->base; > > u32 reg_iflag1, reg_esr; > > > > - reg_iflag1 = flexcan_read(®s->iflag1); > > - reg_esr = flexcan_read(®s->esr); > > + reg_iflag1 = priv->read(®s->iflag1); > > + reg_esr = priv->read(®s->esr); > > /* ACK all bus error and state change IRQ sources */ > > if (reg_esr & FLEXCAN_ESR_ALL_INT) > > - flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr); > > + priv->write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr); > > > > /* > > * schedule NAPI in case of: > > @@ -754,16 +763,16 @@ static irqreturn_t flexcan_irq(int irq, void > *dev_id) > > * save them for later use. > > */ > > priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS; > > - flexcan_write(FLEXCAN_IFLAG_DEFAULT & > > + priv->write(FLEXCAN_IFLAG_DEFAULT & > > ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->imask1); > > - flexcan_write(priv->reg_ctrl_default & > ~FLEXCAN_CTRL_ERR_ALL, > > + priv->write(priv->reg_ctrl_default & > > + ~FLEXCAN_CTRL_ERR_ALL, > > ®s->ctrl); > > napi_schedule(&priv->napi); > > } > > > > /* FIFO overflow */ > > if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) { > > - flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, ®s- > >iflag1); > > + priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, > > + ®s->iflag1); > > dev->stats.rx_over_errors++; > > dev->stats.rx_errors++; > > } > > @@ -774,9 +783,9 @@ static irqreturn_t flexcan_irq(int irq, void > *dev_id) > > stats->tx_packets++; > > can_led_event(dev, CAN_LED_EVENT_TX); > > /* after sending a RTR frame mailbox is in RX mode */ > > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > > ®s- > >cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > > - flexcan_write((1 << FLEXCAN_TX_BUF_ID), ®s->iflag1); > > + priv->write((1 << FLEXCAN_TX_BUF_ID), ®s->iflag1); > > netif_wake_queue(dev); > > } > > > > @@ -790,7 +799,7 @@ static void flexcan_set_bittiming(struct net_device > *dev) > > struct flexcan_regs __iomem *regs = priv->base; > > u32 reg; > > > > - reg = flexcan_read(®s->ctrl); > > + reg = priv->read(®s->ctrl); > > reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) | > > FLEXCAN_CTRL_RJW(0x3) | > > FLEXCAN_CTRL_PSEG1(0x7) | @@ -814,11 +823,11 @@ static > > void flexcan_set_bittiming(struct net_device *dev) > > reg |= FLEXCAN_CTRL_SMP; > > > > netdev_info(dev, "writing ctrl=0x%08x\n", reg); > > - flexcan_write(reg, ®s->ctrl); > > + priv->write(reg, ®s->ctrl); > > > > /* print chip status */ > > netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__, > > - flexcan_read(®s->mcr), flexcan_read(®s->ctrl)); > > + priv->read(®s->mcr), priv->read(®s->ctrl)); > > } > > > > /* > > @@ -858,14 +867,14 @@ static int flexcan_chip_start(struct net_device > *dev) > > * disable local echo > > * > > */ > > - reg_mcr = flexcan_read(®s->mcr); > > + reg_mcr = priv->read(®s->mcr); > > reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff); > > reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT | > > FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN | > > FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS | > > FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID); > > netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr); > > - flexcan_write(reg_mcr, ®s->mcr); > > + priv->write(reg_mcr, ®s->mcr); > > > > /* > > * CTRL > > @@ -879,7 +888,7 @@ static int flexcan_chip_start(struct net_device > *dev) > > * enable bus off interrupt > > * (== FLEXCAN_CTRL_ERR_STATE) > > */ > > - reg_ctrl = flexcan_read(®s->ctrl); > > + reg_ctrl = priv->read(®s->ctrl); > > reg_ctrl &= ~FLEXCAN_CTRL_TSYN; > > reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF | > > FLEXCAN_CTRL_ERR_STATE; > > @@ -897,29 +906,29 @@ static int flexcan_chip_start(struct net_device > *dev) > > /* save for later use */ > > priv->reg_ctrl_default = reg_ctrl; > > netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl); > > - flexcan_write(reg_ctrl, ®s->ctrl); > > + priv->write(reg_ctrl, ®s->ctrl); > > > > /* clear and invalidate all mailboxes first */ > > for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->cantxfg); i++) { > > - flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE, > > + priv->write(FLEXCAN_MB_CODE_RX_INACTIVE, > > ®s->cantxfg[i].can_ctrl); > > } > > > > /* Errata ERR005829: mark first TX mailbox as INACTIVE */ > > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > > ®s->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl); > > > > /* mark TX mailbox as INACTIVE */ > > - flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE, > > + priv->write(FLEXCAN_MB_CODE_TX_INACTIVE, > > ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); > > > > /* acceptance mask/acceptance code (accept everything) */ > > - flexcan_write(0x0, ®s->rxgmask); > > - flexcan_write(0x0, ®s->rx14mask); > > - flexcan_write(0x0, ®s->rx15mask); > > + priv->write(0x0, ®s->rxgmask); > > + priv->write(0x0, ®s->rx14mask); > > + priv->write(0x0, ®s->rx15mask); > > > > if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) > > - flexcan_write(0x0, ®s->rxfgmask); > > + priv->write(0x0, ®s->rxfgmask); > > > > /* > > * On Vybrid, disable memory error detection interrupts @@ > > -934,16 +943,16 @@ static int flexcan_chip_start(struct net_device > *dev) > > * and Correction of Memory Errors" to write to > > * MECR register > > */ > > - reg_crl2 = flexcan_read(®s->crl2); > > + reg_crl2 = priv->read(®s->crl2); > > reg_crl2 |= FLEXCAN_CRL2_ECRWRE; > > - flexcan_write(reg_crl2, ®s->crl2); > > + priv->write(reg_crl2, ®s->crl2); > > > > - reg_mecr = flexcan_read(®s->mecr); > > + reg_mecr = priv->read(®s->mecr); > > reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS; > > - flexcan_write(reg_mecr, ®s->mecr); > > + priv->write(reg_mecr, ®s->mecr); > > reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ | > FLEXCAN_MECR_HANCEI_MSK | > > FLEXCAN_MECR_FANCEI_MSK); > > - flexcan_write(reg_mecr, ®s->mecr); > > + priv->write(reg_mecr, ®s->mecr); > > } > > > > err = flexcan_transceiver_enable(priv); @@ -958,11 +967,11 @@ > > static int flexcan_chip_start(struct net_device *dev) > > priv->can.state = CAN_STATE_ERROR_ACTIVE; > > > > /* enable FIFO interrupts */ > > - flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > > + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); > > > > /* print chip status */ > > netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__, > > - flexcan_read(®s->mcr), flexcan_read(®s->ctrl)); > > + priv->read(®s->mcr), priv->read(®s->ctrl)); > > > > return 0; > > > > @@ -989,8 +998,8 @@ static void flexcan_chip_stop(struct net_device > *dev) > > flexcan_chip_disable(priv); > > > > /* Disable all interrupts */ > > - flexcan_write(0, ®s->imask1); > > - flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, > > + priv->write(0, ®s->imask1); > > + priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, > > ®s->ctrl); > > > > flexcan_transceiver_disable(priv); > > @@ -1108,26 +1117,26 @@ static int register_flexcandev(struct > net_device *dev) > > err = flexcan_chip_disable(priv); > > if (err) > > goto out_disable_per; > > - reg = flexcan_read(®s->ctrl); > > + reg = priv->read(®s->ctrl); > > reg |= FLEXCAN_CTRL_CLK_SRC; > > - flexcan_write(reg, ®s->ctrl); > > + priv->write(reg, ®s->ctrl); > > > > err = flexcan_chip_enable(priv); > > if (err) > > goto out_chip_disable; > > > > /* set freeze, halt and activate FIFO, restrict register access > */ > > - reg = flexcan_read(®s->mcr); > > + reg = priv->read(®s->mcr); > > reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | > > FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV; > > - flexcan_write(reg, ®s->mcr); > > + priv->write(reg, ®s->mcr); > > > > /* > > * Currently we only support newer versions of this core > > * featuring a RX FIFO. Older cores found on some Coldfire > > * derivates are not yet supported. > > */ > > - reg = flexcan_read(®s->mcr); > > + reg = priv->read(®s->mcr); > > if (!(reg & FLEXCAN_MCR_FEN)) { > > netdev_err(dev, "Could not enable RX FIFO, unsupported > core\n"); > > err = -ENODEV; > > @@ -1183,6 +1192,8 @@ static int flexcan_probe(struct platform_device > *pdev) > > void __iomem *base; > > int err, irq; > > u32 clock_freq = 0; > > + /* Default case for most ARM based FSL SoC having BE FlexCAN IP > */ > > + bool core_is_little = true, module_is_little = false; > > > > reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver"); > > if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER) @@ -1237,6 +1248,25 > > @@ static int flexcan_probe(struct platform_device *pdev) > > dev->flags |= IFF_ECHO; > > > > priv = netdev_priv(dev); > > + > > + if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) > > + core_is_little = false; > > Why not just using IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) directly ? > (anyways, cpu_is_le seems a more appropriate name) > > > + if (of_property_read_bool(dev->dev.of_node, "little-endian")) > > + module_is_little = true; > > Why that misleading name, instead of, eg. "chip_is_le" ? > > Oh, and you'll expect explicitly set a new DTB attribute, if the chip is > in LE - the default case. > > Didn't you just say, you dont wanna break anything ? > > Can you imagine that some people use DTB as a _board_ config (eg. in > bootloader) instead of externalized kernel config ? ;-o > > > + if ((core_is_little && module_is_little) || > > + (!core_is_little && !module_is_little)) { > > + priv->read = flexcan_read_le; > > + priv->write = flexcan_write_le; > > + } > > + > > + if ((!core_is_little && module_is_little) || > > + (core_is_little && !module_is_little)) { > > + priv->read = flexcan_read_be; > > + priv->write = flexcan_write_be; > > + } > > At that point, the naming gets completely wrong - instead should be > something like flexcan_read_swab / flexcan_write_swab. > And the decision can easily be made on: > > IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) != module_is_little > > > Putting these together, I'd rather: > > #1: add a new feature flag: > #define FLEXCAN_HAS_BIG_ENDIAN BIT(4) > > #2: add a new dts device type for the new chip variant > (eg. fsl,flexcan-qorliq) > > #2: pass the priv pointer to the *_read()/*_write() functions, > so they can pick up the BE flag and decide whether to > swab or not (based on cpu endianess) > You seem to have missed the endian property discussions that happened regarding DTS Several months back. Have a look at them: [1] & [2]. This patch is compliant to various DTS that already have such a property. And BTW, I have already mentioned this during the v1 review of this patchset: We are dealing with 4 possible cases here: 1. Core is LE (ARM) <-> FlexCAN IP is BE = FSL LS1021A SoC 2. Core is BE (PPC) <-> FlexCAN IP is BE = FSL P1010 SoC 3. Core is BE (PPC) <-> FlexCAN IP is LE = No SoC with this configuration yet 4. Core is LE (PPC) <-> FlexCAN IP is LE = FSL I.MX series So, no this _simply_ won't work the way you described for the above cases. I remember testing this patchset also on a P1010, so I don't find any legacy stuff breaking. [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/254533.html [2] http://lxr.free-electrons.com/source/arch/arm/boot/dts/ls1021a.dtsi#L127 Regards, Bhupesh ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode 2015-05-14 11:33 ` Bhupesh Sharma @ 2015-05-14 11:33 ` Bhupesh Sharma -1 siblings, 0 replies; 41+ messages in thread From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw) To: arnd, linux-can, mkl Cc: bhupesh.sharma, bhupesh.linux, linux-arm-kernel, Sakar.Arora This patch adds support for non RX-FIFO (legacy) mode in the flexcan driver. On certain SoCs, the RX-FIFO support might be broken, as a result we need to fall-back on the legacy (non RX-FIFO) mode to receive CAN frames. Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> --- drivers/net/can/flexcan.c | 188 +++++++++++++++++++++++++++++++++------------ 1 file changed, 138 insertions(+), 50 deletions(-) diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index b0222ae..3240472 100644 --- a/drivers/net/can/flexcan.c +++ b/drivers/net/can/flexcan.c @@ -156,6 +156,9 @@ #define FLEXCAN_IFLAG_DEFAULT \ (FLEXCAN_IFLAG_RX_FIFO_OVERFLOW | FLEXCAN_IFLAG_RX_FIFO_AVAILABLE | \ FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID)) +#define FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE \ + (FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID)) +#define FLEXCAN_IFLAG_RX_MB_RXMASK ((1 << FLEXCAN_TX_BUF_RESERVED) - 1) /* FLEXCAN message buffers */ #define FLEXCAN_MB_CNT_CODE(x) (((x) & 0xf) << 24) @@ -198,6 +201,8 @@ #define FLEXCAN_HAS_V10_FEATURES BIT(1) /* For core version >= 10 */ #define FLEXCAN_HAS_BROKEN_ERR_STATE BIT(2) /* [TR]WRN_INT not connected */ #define FLEXCAN_HAS_MECR_FEATURES BIT(3) /* Memory error detection */ +#define FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT \ + BIT(4) /* No RX FIFO mode support */ /* Structure of the message buffer */ struct flexcan_mb { @@ -252,6 +257,7 @@ struct flexcan_priv { void __iomem *base; u32 reg_esr; u32 reg_ctrl_default; + u32 rx_msg_buf; struct clk *clk_ipg; struct clk *clk_per; @@ -303,8 +309,7 @@ static const struct can_bittiming_const flexcan_bittiming_const = { .brp_inc = 1, }; -/* - * FlexCAN module is essentially modelled as a little-endian IP in most +/* FlexCAN module is essentially modelled as a little-endian IP in most * SoCs, i.e the registers as well as the message buffer areas are * implemented in a little-endian fashion. * @@ -646,12 +651,10 @@ static int flexcan_poll_state(struct net_device *dev, u32 reg_esr) return 1; } -static void flexcan_read_fifo(const struct net_device *dev, - struct can_frame *cf) +static void flexcan_read_can_frame(const struct flexcan_priv *priv, + struct flexcan_mb __iomem *mb, + struct can_frame *cf) { - const struct flexcan_priv *priv = netdev_priv(dev); - struct flexcan_regs __iomem *regs = priv->base; - struct flexcan_mb __iomem *mb = ®s->cantxfg[0]; u32 reg_ctrl, reg_id; reg_ctrl = priv->read(&mb->can_ctrl); @@ -667,14 +670,39 @@ static void flexcan_read_fifo(const struct net_device *dev, *(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb->data[0])); *(__be32 *)(cf->data + 4) = cpu_to_be32(priv->read(&mb->data[1])); +} + +static void flexcan_read_fifo(const struct net_device *dev, + struct can_frame *cf) +{ + const struct flexcan_priv *priv = netdev_priv(dev); + struct flexcan_regs __iomem *regs = priv->base; + struct flexcan_mb __iomem *mb = ®s->cantxfg[0]; + + flexcan_read_can_frame(priv, mb, cf); /* mark as read */ priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); priv->read(®s->timer); } +static void flexcan_read_msg_buf(const struct net_device *dev, + struct can_frame *cf, u32 msg_buf) +{ + const struct flexcan_priv *priv = netdev_priv(dev); + struct flexcan_regs __iomem *regs = priv->base; + struct flexcan_mb __iomem *mb = ®s->cantxfg[msg_buf]; + + flexcan_read_can_frame(priv, mb, cf); + + /* mark as read */ + priv->write(BIT(msg_buf), ®s->iflag1); + priv->read(®s->timer); +} + static int flexcan_read_frame(struct net_device *dev) { + const struct flexcan_priv *priv = netdev_priv(dev); struct net_device_stats *stats = &dev->stats; struct can_frame *cf; struct sk_buff *skb; @@ -685,7 +713,11 @@ static int flexcan_read_frame(struct net_device *dev) return 0; } - flexcan_read_fifo(dev, cf); + if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) + flexcan_read_msg_buf(dev, cf, priv->rx_msg_buf); + else + flexcan_read_fifo(dev, cf); + netif_receive_skb(skb); stats->rx_packets++; @@ -699,9 +731,10 @@ static int flexcan_read_frame(struct net_device *dev) static int flexcan_poll(struct napi_struct *napi, int quota) { struct net_device *dev = napi->dev; - const struct flexcan_priv *priv = netdev_priv(dev); + struct flexcan_priv *priv = netdev_priv(dev); struct flexcan_regs __iomem *regs = priv->base; u32 reg_iflag1, reg_esr; + unsigned long iflag1; int work_done = 0; /* @@ -713,12 +746,25 @@ static int flexcan_poll(struct napi_struct *napi, int quota) /* handle state changes */ work_done += flexcan_poll_state(dev, reg_esr); - /* handle RX-FIFO */ reg_iflag1 = priv->read(®s->iflag1); - while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE && - work_done < quota) { - work_done += flexcan_read_frame(dev); - reg_iflag1 = priv->read(®s->iflag1); + + if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) { + /* handle legacy RX mode */ + iflag1 = reg_iflag1 & FLEXCAN_IFLAG_RX_MB_RXMASK; + while ((reg_iflag1 & FLEXCAN_IFLAG_RX_MB_RXMASK) && + work_done < quota) { + priv->rx_msg_buf = find_first_bit(&iflag1, + (FLEXCAN_TX_BUF_ID - 1)); + work_done += flexcan_read_frame(dev); + reg_iflag1 = priv->read(®s->iflag1); + } + } else { + /* handle RX-FIFO */ + while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE && + work_done < quota) { + work_done += flexcan_read_frame(dev); + reg_iflag1 = priv->read(®s->iflag1); + } } /* report bus errors */ @@ -728,7 +774,13 @@ static int flexcan_poll(struct napi_struct *napi, int quota) if (work_done < quota) { napi_complete(napi); /* enable IRQs */ - priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); + if (priv->devtype_data->features & + FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) + priv->write(FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE | + FLEXCAN_IFLAG_RX_MB_RXMASK, ®s->imask1); + else + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); + priv->write(priv->reg_ctrl_default, ®s->ctrl); } @@ -755,26 +807,45 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) * - state change IRQ * - bus error IRQ and bus error reporting is activated */ - if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) || - (reg_esr & FLEXCAN_ESR_ERR_STATE) || - flexcan_has_and_handle_berr(priv, reg_esr)) { - /* - * The error bits are cleared on read, - * save them for later use. - */ - priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS; - priv->write(FLEXCAN_IFLAG_DEFAULT & - ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->imask1); - priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, - ®s->ctrl); - napi_schedule(&priv->napi); - } + if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) { + if ((reg_iflag1 & FLEXCAN_IFLAG_RX_MB_RXMASK) || + (reg_esr & FLEXCAN_ESR_ERR_STATE) || + flexcan_has_and_handle_berr(priv, reg_esr)) { + /* The error bits are cleared on read, + * save them for later use. + */ + priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS; + priv->write(FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE & + ~FLEXCAN_IFLAG_RX_MB_RXMASK, ®s->imask1); + priv->write(priv->reg_ctrl_default & + ~FLEXCAN_CTRL_ERR_ALL, ®s->ctrl); + + napi_schedule(&priv->napi); + } + } else { + if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) || + (reg_esr & FLEXCAN_ESR_ERR_STATE) || + flexcan_has_and_handle_berr(priv, reg_esr)) { + /* + * The error bits are cleared on read, + * save them for later use. + */ + priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS; + priv->write(FLEXCAN_IFLAG_DEFAULT & + ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, + ®s->imask1); + priv->write(priv->reg_ctrl_default & + ~FLEXCAN_CTRL_ERR_ALL, ®s->ctrl); + napi_schedule(&priv->napi); + } - /* FIFO overflow */ - if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) { - priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, ®s->iflag1); - dev->stats.rx_over_errors++; - dev->stats.rx_errors++; + /* FIFO overflow */ + if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) { + priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, + ®s->iflag1); + dev->stats.rx_over_errors++; + dev->stats.rx_errors++; + } } /* transmission complete interrupt */ @@ -869,7 +940,12 @@ static int flexcan_chip_start(struct net_device *dev) */ reg_mcr = priv->read(®s->mcr); reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff); - reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT | + if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) + reg_mcr &= ~FLEXCAN_MCR_FEN; + else + reg_mcr |= FLEXCAN_MCR_FEN; + + reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN | FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS | FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID); @@ -966,8 +1042,13 @@ static int flexcan_chip_start(struct net_device *dev) priv->can.state = CAN_STATE_ERROR_ACTIVE; - /* enable FIFO interrupts */ - priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); + if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) + /* enable mb interrupts */ + priv->write(FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE | + FLEXCAN_IFLAG_RX_MB_RXMASK, ®s->imask1); + else + /* enable FIFO interrupts */ + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); /* print chip status */ netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__, @@ -1125,22 +1206,29 @@ static int register_flexcandev(struct net_device *dev) if (err) goto out_chip_disable; - /* set freeze, halt and activate FIFO, restrict register access */ + /* set freeze, halt and activate FIFO/legacy mode, restrict + * register access + */ reg = priv->read(®s->mcr); - reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | - FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV; + if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) + reg &= ~FLEXCAN_MCR_FEN; + else + reg |= FLEXCAN_MCR_FEN; + + reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | FLEXCAN_MCR_SUPV; priv->write(reg, ®s->mcr); - /* - * Currently we only support newer versions of this core - * featuring a RX FIFO. Older cores found on some Coldfire - * derivates are not yet supported. - */ - reg = priv->read(®s->mcr); - if (!(reg & FLEXCAN_MCR_FEN)) { - netdev_err(dev, "Could not enable RX FIFO, unsupported core\n"); - err = -ENODEV; - goto out_chip_disable; + if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) { + /* Legacy RX mode*/ + netdev_info(dev, "Legacy mode (non RX-FIFO) enabled\n"); + } else { + /* RX FIFO mode */ + reg = priv->read(®s->mcr); + if (!(reg & FLEXCAN_MCR_FEN)) { + netdev_err(dev, "Could not enable RX FIFO, unsupported core\n"); + err = -ENODEV; + goto out_chip_disable; + } } err = register_candev(dev); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode @ 2015-05-14 11:33 ` Bhupesh Sharma 0 siblings, 0 replies; 41+ messages in thread From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw) To: linux-arm-kernel This patch adds support for non RX-FIFO (legacy) mode in the flexcan driver. On certain SoCs, the RX-FIFO support might be broken, as a result we need to fall-back on the legacy (non RX-FIFO) mode to receive CAN frames. Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> --- drivers/net/can/flexcan.c | 188 +++++++++++++++++++++++++++++++++------------ 1 file changed, 138 insertions(+), 50 deletions(-) diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index b0222ae..3240472 100644 --- a/drivers/net/can/flexcan.c +++ b/drivers/net/can/flexcan.c @@ -156,6 +156,9 @@ #define FLEXCAN_IFLAG_DEFAULT \ (FLEXCAN_IFLAG_RX_FIFO_OVERFLOW | FLEXCAN_IFLAG_RX_FIFO_AVAILABLE | \ FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID)) +#define FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE \ + (FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID)) +#define FLEXCAN_IFLAG_RX_MB_RXMASK ((1 << FLEXCAN_TX_BUF_RESERVED) - 1) /* FLEXCAN message buffers */ #define FLEXCAN_MB_CNT_CODE(x) (((x) & 0xf) << 24) @@ -198,6 +201,8 @@ #define FLEXCAN_HAS_V10_FEATURES BIT(1) /* For core version >= 10 */ #define FLEXCAN_HAS_BROKEN_ERR_STATE BIT(2) /* [TR]WRN_INT not connected */ #define FLEXCAN_HAS_MECR_FEATURES BIT(3) /* Memory error detection */ +#define FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT \ + BIT(4) /* No RX FIFO mode support */ /* Structure of the message buffer */ struct flexcan_mb { @@ -252,6 +257,7 @@ struct flexcan_priv { void __iomem *base; u32 reg_esr; u32 reg_ctrl_default; + u32 rx_msg_buf; struct clk *clk_ipg; struct clk *clk_per; @@ -303,8 +309,7 @@ static const struct can_bittiming_const flexcan_bittiming_const = { .brp_inc = 1, }; -/* - * FlexCAN module is essentially modelled as a little-endian IP in most +/* FlexCAN module is essentially modelled as a little-endian IP in most * SoCs, i.e the registers as well as the message buffer areas are * implemented in a little-endian fashion. * @@ -646,12 +651,10 @@ static int flexcan_poll_state(struct net_device *dev, u32 reg_esr) return 1; } -static void flexcan_read_fifo(const struct net_device *dev, - struct can_frame *cf) +static void flexcan_read_can_frame(const struct flexcan_priv *priv, + struct flexcan_mb __iomem *mb, + struct can_frame *cf) { - const struct flexcan_priv *priv = netdev_priv(dev); - struct flexcan_regs __iomem *regs = priv->base; - struct flexcan_mb __iomem *mb = ®s->cantxfg[0]; u32 reg_ctrl, reg_id; reg_ctrl = priv->read(&mb->can_ctrl); @@ -667,14 +670,39 @@ static void flexcan_read_fifo(const struct net_device *dev, *(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb->data[0])); *(__be32 *)(cf->data + 4) = cpu_to_be32(priv->read(&mb->data[1])); +} + +static void flexcan_read_fifo(const struct net_device *dev, + struct can_frame *cf) +{ + const struct flexcan_priv *priv = netdev_priv(dev); + struct flexcan_regs __iomem *regs = priv->base; + struct flexcan_mb __iomem *mb = ®s->cantxfg[0]; + + flexcan_read_can_frame(priv, mb, cf); /* mark as read */ priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); priv->read(®s->timer); } +static void flexcan_read_msg_buf(const struct net_device *dev, + struct can_frame *cf, u32 msg_buf) +{ + const struct flexcan_priv *priv = netdev_priv(dev); + struct flexcan_regs __iomem *regs = priv->base; + struct flexcan_mb __iomem *mb = ®s->cantxfg[msg_buf]; + + flexcan_read_can_frame(priv, mb, cf); + + /* mark as read */ + priv->write(BIT(msg_buf), ®s->iflag1); + priv->read(®s->timer); +} + static int flexcan_read_frame(struct net_device *dev) { + const struct flexcan_priv *priv = netdev_priv(dev); struct net_device_stats *stats = &dev->stats; struct can_frame *cf; struct sk_buff *skb; @@ -685,7 +713,11 @@ static int flexcan_read_frame(struct net_device *dev) return 0; } - flexcan_read_fifo(dev, cf); + if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) + flexcan_read_msg_buf(dev, cf, priv->rx_msg_buf); + else + flexcan_read_fifo(dev, cf); + netif_receive_skb(skb); stats->rx_packets++; @@ -699,9 +731,10 @@ static int flexcan_read_frame(struct net_device *dev) static int flexcan_poll(struct napi_struct *napi, int quota) { struct net_device *dev = napi->dev; - const struct flexcan_priv *priv = netdev_priv(dev); + struct flexcan_priv *priv = netdev_priv(dev); struct flexcan_regs __iomem *regs = priv->base; u32 reg_iflag1, reg_esr; + unsigned long iflag1; int work_done = 0; /* @@ -713,12 +746,25 @@ static int flexcan_poll(struct napi_struct *napi, int quota) /* handle state changes */ work_done += flexcan_poll_state(dev, reg_esr); - /* handle RX-FIFO */ reg_iflag1 = priv->read(®s->iflag1); - while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE && - work_done < quota) { - work_done += flexcan_read_frame(dev); - reg_iflag1 = priv->read(®s->iflag1); + + if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) { + /* handle legacy RX mode */ + iflag1 = reg_iflag1 & FLEXCAN_IFLAG_RX_MB_RXMASK; + while ((reg_iflag1 & FLEXCAN_IFLAG_RX_MB_RXMASK) && + work_done < quota) { + priv->rx_msg_buf = find_first_bit(&iflag1, + (FLEXCAN_TX_BUF_ID - 1)); + work_done += flexcan_read_frame(dev); + reg_iflag1 = priv->read(®s->iflag1); + } + } else { + /* handle RX-FIFO */ + while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE && + work_done < quota) { + work_done += flexcan_read_frame(dev); + reg_iflag1 = priv->read(®s->iflag1); + } } /* report bus errors */ @@ -728,7 +774,13 @@ static int flexcan_poll(struct napi_struct *napi, int quota) if (work_done < quota) { napi_complete(napi); /* enable IRQs */ - priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); + if (priv->devtype_data->features & + FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) + priv->write(FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE | + FLEXCAN_IFLAG_RX_MB_RXMASK, ®s->imask1); + else + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); + priv->write(priv->reg_ctrl_default, ®s->ctrl); } @@ -755,26 +807,45 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) * - state change IRQ * - bus error IRQ and bus error reporting is activated */ - if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) || - (reg_esr & FLEXCAN_ESR_ERR_STATE) || - flexcan_has_and_handle_berr(priv, reg_esr)) { - /* - * The error bits are cleared on read, - * save them for later use. - */ - priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS; - priv->write(FLEXCAN_IFLAG_DEFAULT & - ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->imask1); - priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, - ®s->ctrl); - napi_schedule(&priv->napi); - } + if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) { + if ((reg_iflag1 & FLEXCAN_IFLAG_RX_MB_RXMASK) || + (reg_esr & FLEXCAN_ESR_ERR_STATE) || + flexcan_has_and_handle_berr(priv, reg_esr)) { + /* The error bits are cleared on read, + * save them for later use. + */ + priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS; + priv->write(FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE & + ~FLEXCAN_IFLAG_RX_MB_RXMASK, ®s->imask1); + priv->write(priv->reg_ctrl_default & + ~FLEXCAN_CTRL_ERR_ALL, ®s->ctrl); + + napi_schedule(&priv->napi); + } + } else { + if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) || + (reg_esr & FLEXCAN_ESR_ERR_STATE) || + flexcan_has_and_handle_berr(priv, reg_esr)) { + /* + * The error bits are cleared on read, + * save them for later use. + */ + priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS; + priv->write(FLEXCAN_IFLAG_DEFAULT & + ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, + ®s->imask1); + priv->write(priv->reg_ctrl_default & + ~FLEXCAN_CTRL_ERR_ALL, ®s->ctrl); + napi_schedule(&priv->napi); + } - /* FIFO overflow */ - if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) { - priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, ®s->iflag1); - dev->stats.rx_over_errors++; - dev->stats.rx_errors++; + /* FIFO overflow */ + if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) { + priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, + ®s->iflag1); + dev->stats.rx_over_errors++; + dev->stats.rx_errors++; + } } /* transmission complete interrupt */ @@ -869,7 +940,12 @@ static int flexcan_chip_start(struct net_device *dev) */ reg_mcr = priv->read(®s->mcr); reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff); - reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT | + if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) + reg_mcr &= ~FLEXCAN_MCR_FEN; + else + reg_mcr |= FLEXCAN_MCR_FEN; + + reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN | FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS | FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID); @@ -966,8 +1042,13 @@ static int flexcan_chip_start(struct net_device *dev) priv->can.state = CAN_STATE_ERROR_ACTIVE; - /* enable FIFO interrupts */ - priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); + if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) + /* enable mb interrupts */ + priv->write(FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE | + FLEXCAN_IFLAG_RX_MB_RXMASK, ®s->imask1); + else + /* enable FIFO interrupts */ + priv->write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); /* print chip status */ netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__, @@ -1125,22 +1206,29 @@ static int register_flexcandev(struct net_device *dev) if (err) goto out_chip_disable; - /* set freeze, halt and activate FIFO, restrict register access */ + /* set freeze, halt and activate FIFO/legacy mode, restrict + * register access + */ reg = priv->read(®s->mcr); - reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | - FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV; + if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) + reg &= ~FLEXCAN_MCR_FEN; + else + reg |= FLEXCAN_MCR_FEN; + + reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | FLEXCAN_MCR_SUPV; priv->write(reg, ®s->mcr); - /* - * Currently we only support newer versions of this core - * featuring a RX FIFO. Older cores found on some Coldfire - * derivates are not yet supported. - */ - reg = priv->read(®s->mcr); - if (!(reg & FLEXCAN_MCR_FEN)) { - netdev_err(dev, "Could not enable RX FIFO, unsupported core\n"); - err = -ENODEV; - goto out_chip_disable; + if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) { + /* Legacy RX mode*/ + netdev_info(dev, "Legacy mode (non RX-FIFO) enabled\n"); + } else { + /* RX FIFO mode */ + reg = priv->read(®s->mcr); + if (!(reg & FLEXCAN_MCR_FEN)) { + netdev_err(dev, "Could not enable RX FIFO, unsupported core\n"); + err = -ENODEV; + goto out_chip_disable; + } } err = register_candev(dev); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode 2015-05-14 11:33 ` Bhupesh Sharma @ 2015-05-14 15:41 ` Marc Kleine-Budde -1 siblings, 0 replies; 41+ messages in thread From: Marc Kleine-Budde @ 2015-05-14 15:41 UTC (permalink / raw) To: Bhupesh Sharma, arnd, linux-can Cc: bhupesh.linux, Sakar.Arora, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 944 bytes --] On 05/14/2015 01:33 PM, Bhupesh Sharma wrote: > This patch adds support for non RX-FIFO (legacy) mode in > the flexcan driver. > > On certain SoCs, the RX-FIFO support might be broken, as > a result we need to fall-back on the legacy (non RX-FIFO) > mode to receive CAN frames. > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> > Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> The non FIFO mode doesn't guarantee the order of the incoming frames, not does not even try to...this is not acceptable. I'm currently working on a patch by David Jander that brings in non FIFO mode, but tries to keep the order of the frames. 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: 801 bytes --] ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode @ 2015-05-14 15:41 ` Marc Kleine-Budde 0 siblings, 0 replies; 41+ messages in thread From: Marc Kleine-Budde @ 2015-05-14 15:41 UTC (permalink / raw) To: linux-arm-kernel On 05/14/2015 01:33 PM, Bhupesh Sharma wrote: > This patch adds support for non RX-FIFO (legacy) mode in > the flexcan driver. > > On certain SoCs, the RX-FIFO support might be broken, as > a result we need to fall-back on the legacy (non RX-FIFO) > mode to receive CAN frames. > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> > Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> The non FIFO mode doesn't guarantee the order of the incoming frames, not does not even try to...this is not acceptable. I'm currently working on a patch by David Jander that brings in non FIFO mode, but tries to keep the order of the frames. 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 | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: OpenPGP digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150514/f269ccab/attachment.sig> ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode 2015-05-14 15:41 ` Marc Kleine-Budde @ 2015-05-14 15:44 ` Sharma Bhupesh -1 siblings, 0 replies; 41+ messages in thread From: Sharma Bhupesh @ 2015-05-14 15:44 UTC (permalink / raw) To: Marc Kleine-Budde, arnd, linux-can Cc: bhupesh.linux, Arora Sakar, linux-arm-kernel > -----Original Message----- > From: Marc Kleine-Budde [mailto:mkl@pengutronix.de] > Sent: Thursday, May 14, 2015 9:12 PM > To: Sharma Bhupesh-B45370; arnd@arndb.de; linux-can@vger.kernel.org > Cc: bhupesh.linux@gmail.com; Arora Sakar-B45205; linux-arm- > kernel@lists.infradead.org > Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO > mode > > On 05/14/2015 01:33 PM, Bhupesh Sharma wrote: > > This patch adds support for non RX-FIFO (legacy) mode in the flexcan > > driver. > > > > On certain SoCs, the RX-FIFO support might be broken, as a result we > > need to fall-back on the legacy (non RX-FIFO) mode to receive CAN > > frames. > > > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> > > Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> > > The non FIFO mode doesn't guarantee the order of the incoming frames, not > does not even try to...this is not acceptable. I'm currently working on a > patch by David Jander that brings in non FIFO mode, but tries to keep the > order of the frames. That is already WIP at our end. V3 will contain the same change. If you are already working on it, I don't know how to proceed further as we had already v1 of this patchset with the non FIFO mode out since a month or so. Regards, Bhupesh ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode @ 2015-05-14 15:44 ` Sharma Bhupesh 0 siblings, 0 replies; 41+ messages in thread From: Sharma Bhupesh @ 2015-05-14 15:44 UTC (permalink / raw) To: linux-arm-kernel > -----Original Message----- > From: Marc Kleine-Budde [mailto:mkl at pengutronix.de] > Sent: Thursday, May 14, 2015 9:12 PM > To: Sharma Bhupesh-B45370; arnd at arndb.de; linux-can at vger.kernel.org > Cc: bhupesh.linux at gmail.com; Arora Sakar-B45205; linux-arm- > kernel at lists.infradead.org > Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO > mode > > On 05/14/2015 01:33 PM, Bhupesh Sharma wrote: > > This patch adds support for non RX-FIFO (legacy) mode in the flexcan > > driver. > > > > On certain SoCs, the RX-FIFO support might be broken, as a result we > > need to fall-back on the legacy (non RX-FIFO) mode to receive CAN > > frames. > > > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> > > Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> > > The non FIFO mode doesn't guarantee the order of the incoming frames, not > does not even try to...this is not acceptable. I'm currently working on a > patch by David Jander that brings in non FIFO mode, but tries to keep the > order of the frames. That is already WIP at our end. V3 will contain the same change. If you are already working on it, I don't know how to proceed further as we had already v1 of this patchset with the non FIFO mode out since a month or so. Regards, Bhupesh ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode 2015-05-14 15:44 ` Sharma Bhupesh @ 2015-12-10 11:05 ` Sharma Bhupesh -1 siblings, 0 replies; 41+ messages in thread From: Sharma Bhupesh @ 2015-12-10 11:05 UTC (permalink / raw) To: Marc Kleine-Budde, arnd, linux-can Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar Hi Mark, > -----Original Message----- > From: linux-arm-kernel [mailto:linux-arm-kernel- > bounces@lists.infradead.org] On Behalf Of Sharma Bhupesh > Sent: Thursday, May 14, 2015 9:14 PM > To: Marc Kleine-Budde; arnd@arndb.de; linux-can@vger.kernel.org > Cc: bhupesh.linux@gmail.com; linux-arm-kernel@lists.infradead.org; Arora > Sakar-B45205 > Subject: RE: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO > mode > > > -----Original Message----- > > From: Marc Kleine-Budde [mailto:mkl@pengutronix.de] > > Sent: Thursday, May 14, 2015 9:12 PM > > To: Sharma Bhupesh-B45370; arnd@arndb.de; linux-can@vger.kernel.org > > Cc: bhupesh.linux@gmail.com; Arora Sakar-B45205; linux-arm- > > kernel@lists.infradead.org > > Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO > > mode > > > > On 05/14/2015 01:33 PM, Bhupesh Sharma wrote: > > > This patch adds support for non RX-FIFO (legacy) mode in the flexcan > > > driver. > > > > > > On certain SoCs, the RX-FIFO support might be broken, as a result we > > > need to fall-back on the legacy (non RX-FIFO) mode to receive CAN > > > frames. > > > > > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> > > > Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> > > > > The non FIFO mode doesn't guarantee the order of the incoming frames, > > not does not even try to...this is not acceptable. I'm currently > > working on a patch by David Jander that brings in non FIFO mode, but > > tries to keep the order of the frames. > > That is already WIP at our end. V3 will contain the same change. > If you are already working on it, I don't know how to proceed further as > we had already v1 of this patchset with the non FIFO mode out since a > month or so. I don't remember seeing a patch from you which supports non-FIFO mode for flexcan IP. Since we now have Freescale LS1021A chip out in the field on which the FIFO mode broken, we cannot use the upstream flexcan driver for enabling flexcan IP on these chips. Do you still have plans to work on this? Or should I submit my v3 with in-order reception supported for rx frames. Regards, Bhupesh ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode @ 2015-12-10 11:05 ` Sharma Bhupesh 0 siblings, 0 replies; 41+ messages in thread From: Sharma Bhupesh @ 2015-12-10 11:05 UTC (permalink / raw) To: linux-arm-kernel Hi Mark, > -----Original Message----- > From: linux-arm-kernel [mailto:linux-arm-kernel- > bounces at lists.infradead.org] On Behalf Of Sharma Bhupesh > Sent: Thursday, May 14, 2015 9:14 PM > To: Marc Kleine-Budde; arnd at arndb.de; linux-can at vger.kernel.org > Cc: bhupesh.linux at gmail.com; linux-arm-kernel at lists.infradead.org; Arora > Sakar-B45205 > Subject: RE: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO > mode > > > -----Original Message----- > > From: Marc Kleine-Budde [mailto:mkl at pengutronix.de] > > Sent: Thursday, May 14, 2015 9:12 PM > > To: Sharma Bhupesh-B45370; arnd at arndb.de; linux-can at vger.kernel.org > > Cc: bhupesh.linux at gmail.com; Arora Sakar-B45205; linux-arm- > > kernel at lists.infradead.org > > Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO > > mode > > > > On 05/14/2015 01:33 PM, Bhupesh Sharma wrote: > > > This patch adds support for non RX-FIFO (legacy) mode in the flexcan > > > driver. > > > > > > On certain SoCs, the RX-FIFO support might be broken, as a result we > > > need to fall-back on the legacy (non RX-FIFO) mode to receive CAN > > > frames. > > > > > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> > > > Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> > > > > The non FIFO mode doesn't guarantee the order of the incoming frames, > > not does not even try to...this is not acceptable. I'm currently > > working on a patch by David Jander that brings in non FIFO mode, but > > tries to keep the order of the frames. > > That is already WIP at our end. V3 will contain the same change. > If you are already working on it, I don't know how to proceed further as > we had already v1 of this patchset with the non FIFO mode out since a > month or so. I don't remember seeing a patch from you which supports non-FIFO mode for flexcan IP. Since we now have Freescale LS1021A chip out in the field on which the FIFO mode broken, we cannot use the upstream flexcan driver for enabling flexcan IP on these chips. Do you still have plans to work on this? Or should I submit my v3 with in-order reception supported for rx frames. Regards, Bhupesh ^ permalink raw reply [flat|nested] 41+ messages in thread
* can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works. 2015-12-10 11:05 ` Sharma Bhupesh (?) @ 2015-12-10 11:44 ` Tom Evans 2015-12-10 12:44 ` Marc Kleine-Budde -1 siblings, 1 reply; 41+ messages in thread From: Tom Evans @ 2015-12-10 11:44 UTC (permalink / raw) To: Sharma Bhupesh, Marc Kleine-Budde, arnd, linux-can Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar On 10/12/2015 10:05 PM, Sharma Bhupesh wrote: > Hi Mark, >>> Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO >>> mode I've just had to delve back into the deep murky past and try to get Freescale's Kernel 2.6.35 FlexCAN driver working. That's because the only code base that supports Freescale's Video Hardware is that one and we need working video. I'm suggesting this code as an "historical reference" that might be useful for anyone looking for simple/simplistic ways to handle a FlexCAN port. It isn't like the driver actually works fully. It can't have ever been tested very much. Which is why I've had to fix it. It has the Driver, Register handling and Message Buffer handling split into three separate source files, which is a useful simplification. It can't be configured with "canconfig", but exposes a huge number of variables in the sysfs which is actually quite powerful and extendable. Freescale's "2.6.35_maintain" kernel consists of 2.6.35 plus about 1200 Freescale patches which basically replace all the hardware drivers. The source for this driver in this tree (until someone renames it to NXP): http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/log/?h=imx_2.6.35_maintain The Driver is in the expected place: http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/net/can/flexcan?h=imx_2.6.35_maintain 1 - It defaults to 32 receive and 32 transmit MBs, but that can be changed. 2 - It can't receive in order, but you'd only need to sort on the timestamps to fix that (or use the FIFO). 3 - It can't transmit in order either (unless you drop it to one transmit MB), but a simple change to the Transmit MB assignment would fix that. 4 - It supports FIFO mode, but it doesn't work at all. The FIFO hardware gets so badly screwed by the driver that it has to be power-cycled to get it working again. 5 - Transmit doesn't work either. If you push it it'll start sending ONE Message per SECOND. That's an easy fix by changing the code to actually unblock the netif queue instead of what it does. 6 - The sysfs variable that reserves receive message buffers is named to reserve transmit buffers, it messes up if the DLC is over 8 (because it doesn't call get_can_dlc()), the Dump routines don't work properly, off-by-one bug, comparison-reversal bug and so on. 7 - The only good thing about it is that it doesn't use NAPI, so when fixed it doesn't drop messages when you try to use MMC/eMMC/ESDHC. FIFO bug documented and fixed here: https://community.freescale.com/thread/381075 Transmit bug documented and fixed here (and the other ones listed), also MMC/eMMC problem: https://community.freescale.com/message/595897 Patches to fix all of it on the end of this thread: https://community.freescale.com/thread/303271 Tom ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works. 2015-12-10 11:44 ` can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works Tom Evans @ 2015-12-10 12:44 ` Marc Kleine-Budde 0 siblings, 0 replies; 41+ messages in thread From: Marc Kleine-Budde @ 2015-12-10 12:44 UTC (permalink / raw) To: Tom Evans, Sharma Bhupesh, arnd, linux-can Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar [-- Attachment #1: Type: text/plain, Size: 3629 bytes --] On 12/10/2015 12:44 PM, Tom Evans wrote: > I've just had to delve back into the deep murky past and try to get > Freescale's Kernel 2.6.35 FlexCAN driver working. That's because the > only code base that supports Freescale's Video Hardware is that one and > we need working video. 2.6.x I feel your pain. Another option would be to port the flexcan driver to that old kernel. But I'm getting off topic here :) > I'm suggesting this code as an "historical reference" that might be > useful for anyone looking for simple/simplistic ways to handle a FlexCAN > port. It isn't like the driver actually works fully. It can't have ever > been tested very much. Which is why I've had to fix it. > > It has the Driver, Register handling and Message Buffer handling split > into three separate source files, which is a useful simplification. YMMV :) > It can't be configured with "canconfig", but exposes a huge number of > variables in the sysfs which is actually quite powerful and extendable. > > Freescale's "2.6.35_maintain" kernel consists of 2.6.35 plus about 1200 > Freescale patches which basically replace all the hardware drivers. > > The source for this driver in this tree (until someone renames it to NXP): > > http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/log/?h=imx_2.6.35_maintain > > The Driver is in the expected place: > > http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/net/can/flexcan?h=imx_2.6.35_maintain > > 1 - It defaults to 32 receive and 32 transmit MBs, but that can be changed. > > 2 - It can't receive in order, but you'd only need to sort on the > timestamps to fix that (or use the FIFO). In order reception is crucial. > 3 - It can't transmit in order either (unless you drop it to one > transmit MB), but a simple change to the Transmit MB assignment would > fix that. Same here. > 4 - It supports FIFO mode, but it doesn't work at all. The FIFO hardware > gets so badly screwed by the driver that it has to be power-cycled to > get it working again. > > 5 - Transmit doesn't work either. If you push it it'll start sending ONE > Message per SECOND. That's an easy fix by changing the code to actually > unblock the netif queue instead of what it does. > > 6 - The sysfs variable that reserves receive message buffers is named to > reserve transmit buffers, it messes up if the DLC is over 8 (because it > doesn't call get_can_dlc()), the Dump routines don't work properly, > off-by-one bug, comparison-reversal bug and so on. > > 7 - The only good thing about it is that it doesn't use NAPI, so when > fixed it doesn't drop messages when you try to use MMC/eMMC/ESDHC. > > FIFO bug documented and fixed here: > > https://community.freescale.com/thread/381075 > > Transmit bug documented and fixed here (and the other ones listed), also > MMC/eMMC problem: > > https://community.freescale.com/message/595897 > > Patches to fix all of it on the end of this thread: > > https://community.freescale.com/thread/303271 If there's interest in working with this prehistoric kernel, why isn't there anyone that picks up all the patches and add them to a git repo. fsl doesn't seem to care either, maybe unless you're using their newest product :( just my 2¢, 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: 455 bytes --] ^ permalink raw reply [flat|nested] 41+ messages in thread
* can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works. @ 2015-12-10 12:44 ` Marc Kleine-Budde 0 siblings, 0 replies; 41+ messages in thread From: Marc Kleine-Budde @ 2015-12-10 12:44 UTC (permalink / raw) To: linux-arm-kernel On 12/10/2015 12:44 PM, Tom Evans wrote: > I've just had to delve back into the deep murky past and try to get > Freescale's Kernel 2.6.35 FlexCAN driver working. That's because the > only code base that supports Freescale's Video Hardware is that one and > we need working video. 2.6.x I feel your pain. Another option would be to port the flexcan driver to that old kernel. But I'm getting off topic here :) > I'm suggesting this code as an "historical reference" that might be > useful for anyone looking for simple/simplistic ways to handle a FlexCAN > port. It isn't like the driver actually works fully. It can't have ever > been tested very much. Which is why I've had to fix it. > > It has the Driver, Register handling and Message Buffer handling split > into three separate source files, which is a useful simplification. YMMV :) > It can't be configured with "canconfig", but exposes a huge number of > variables in the sysfs which is actually quite powerful and extendable. > > Freescale's "2.6.35_maintain" kernel consists of 2.6.35 plus about 1200 > Freescale patches which basically replace all the hardware drivers. > > The source for this driver in this tree (until someone renames it to NXP): > > http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/log/?h=imx_2.6.35_maintain > > The Driver is in the expected place: > > http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/net/can/flexcan?h=imx_2.6.35_maintain > > 1 - It defaults to 32 receive and 32 transmit MBs, but that can be changed. > > 2 - It can't receive in order, but you'd only need to sort on the > timestamps to fix that (or use the FIFO). In order reception is crucial. > 3 - It can't transmit in order either (unless you drop it to one > transmit MB), but a simple change to the Transmit MB assignment would > fix that. Same here. > 4 - It supports FIFO mode, but it doesn't work at all. The FIFO hardware > gets so badly screwed by the driver that it has to be power-cycled to > get it working again. > > 5 - Transmit doesn't work either. If you push it it'll start sending ONE > Message per SECOND. That's an easy fix by changing the code to actually > unblock the netif queue instead of what it does. > > 6 - The sysfs variable that reserves receive message buffers is named to > reserve transmit buffers, it messes up if the DLC is over 8 (because it > doesn't call get_can_dlc()), the Dump routines don't work properly, > off-by-one bug, comparison-reversal bug and so on. > > 7 - The only good thing about it is that it doesn't use NAPI, so when > fixed it doesn't drop messages when you try to use MMC/eMMC/ESDHC. > > FIFO bug documented and fixed here: > > https://community.freescale.com/thread/381075 > > Transmit bug documented and fixed here (and the other ones listed), also > MMC/eMMC problem: > > https://community.freescale.com/message/595897 > > Patches to fix all of it on the end of this thread: > > https://community.freescale.com/thread/303271 If there's interest in working with this prehistoric kernel, why isn't there anyone that picks up all the patches and add them to a git repo. fsl doesn't seem to care either, maybe unless you're using their newest product :( just my 2?, 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 | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 455 bytes Desc: OpenPGP digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151210/e8f00de9/attachment-0001.sig> ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works. 2015-12-10 12:44 ` Marc Kleine-Budde @ 2015-12-10 22:53 ` Tom Evans -1 siblings, 0 replies; 41+ messages in thread From: Tom Evans @ 2015-12-10 22:53 UTC (permalink / raw) To: Marc Kleine-Budde, Sharma Bhupesh, arnd, linux-can Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar On 10/12/2015 11:44 PM, Marc Kleine-Budde wrote: > On 12/10/2015 12:44 PM, Tom Evans wrote: >> I've just had to delve back into the deep murky past and try to get >> Freescale's Kernel 2.6.35 FlexCAN driver working... On an i.MX53. That's the last supported kernel Freescale ever released for that series of parts. > 2.6.x I feel your pain. Another option would be to port the flexcan > driver to that old kernel. But I'm getting off topic here :) Been there. Options are to port 2.6.38 (Freescale's Android release) which has the 2.6.38 mainstream driver, but looks to have a full collection of all the bugs fixed since then. Some kind soul has already done that and released a patch (but I don't like the bugs). Or the 3.4 version we're running with our 3.4 kernel or something more modern. I don't trust later versions to even compile - the headers and interfaces change. Then I can't use modern ones anyway as they use NAPI and that simply doesn't work with Linux and CAN. So I'd have to apply all my de-NAPI patches. The simplest path was to fix the simple and stupidly obvious bugs in the existing driver rather than add new complicated bugs. >> 2 - It can't receive in order, but you'd only need to sort on the >> 3 - It can't transmit in order either (unless you drop it to one > > In order reception is crucial. So what mind-set at Freescale doesn't see this as a problem? I suspect their CAN knowledge stopped at the HCS08 stage, making tiny devices that control just one thing, like a door lock. So just receive and transmit one or two messages where "priority order" is more important than running any debug or diagnostic protocols. >> Patches to fix all of it on the end of this thread: >> >> https://community.freescale.com/thread/303271 > > If there's interest in working with this prehistoric kernel, why isn't > there anyone that picks up all the patches and add them to a git repo. There was. That's what the above post is meant to be for. But the last actual activity was in January 2013. It details how to submit patches, but it is too onerous for me. It has to use their exact complete distribution (of everything, not just the kernel) running on their exact development board. It also needs your .gitconfig set up "just so" matching your Freescale registration details. I'm using the thread as a dumping ground for "useful patches for customers", and hope google or Freescale's search can find them. > fsl doesn't seem to care either, maybe unless you're using > their newest product :( The team seems to disappear almost before the product is released. They all moved to i.MX6 years ago. Tom ^ permalink raw reply [flat|nested] 41+ messages in thread
* can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works. @ 2015-12-10 22:53 ` Tom Evans 0 siblings, 0 replies; 41+ messages in thread From: Tom Evans @ 2015-12-10 22:53 UTC (permalink / raw) To: linux-arm-kernel On 10/12/2015 11:44 PM, Marc Kleine-Budde wrote: > On 12/10/2015 12:44 PM, Tom Evans wrote: >> I've just had to delve back into the deep murky past and try to get >> Freescale's Kernel 2.6.35 FlexCAN driver working... On an i.MX53. That's the last supported kernel Freescale ever released for that series of parts. > 2.6.x I feel your pain. Another option would be to port the flexcan > driver to that old kernel. But I'm getting off topic here :) Been there. Options are to port 2.6.38 (Freescale's Android release) which has the 2.6.38 mainstream driver, but looks to have a full collection of all the bugs fixed since then. Some kind soul has already done that and released a patch (but I don't like the bugs). Or the 3.4 version we're running with our 3.4 kernel or something more modern. I don't trust later versions to even compile - the headers and interfaces change. Then I can't use modern ones anyway as they use NAPI and that simply doesn't work with Linux and CAN. So I'd have to apply all my de-NAPI patches. The simplest path was to fix the simple and stupidly obvious bugs in the existing driver rather than add new complicated bugs. >> 2 - It can't receive in order, but you'd only need to sort on the >> 3 - It can't transmit in order either (unless you drop it to one > > In order reception is crucial. So what mind-set at Freescale doesn't see this as a problem? I suspect their CAN knowledge stopped at the HCS08 stage, making tiny devices that control just one thing, like a door lock. So just receive and transmit one or two messages where "priority order" is more important than running any debug or diagnostic protocols. >> Patches to fix all of it on the end of this thread: >> >> https://community.freescale.com/thread/303271 > > If there's interest in working with this prehistoric kernel, why isn't > there anyone that picks up all the patches and add them to a git repo. There was. That's what the above post is meant to be for. But the last actual activity was in January 2013. It details how to submit patches, but it is too onerous for me. It has to use their exact complete distribution (of everything, not just the kernel) running on their exact development board. It also needs your .gitconfig set up "just so" matching your Freescale registration details. I'm using the thread as a dumping ground for "useful patches for customers", and hope google or Freescale's search can find them. > fsl doesn't seem to care either, maybe unless you're using > their newest product :( The team seems to disappear almost before the product is released. They all moved to i.MX6 years ago. Tom ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works. 2015-12-10 22:53 ` Tom Evans @ 2015-12-17 4:22 ` Tom Evans -1 siblings, 0 replies; 41+ messages in thread From: Tom Evans @ 2015-12-17 4:22 UTC (permalink / raw) To: Marc Kleine-Budde, Sharma Bhupesh, arnd, linux-can Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar On 11/12/15 09:53, Tom Evans wrote: > On 10/12/2015 11:44 PM, Marc Kleine-Budde wrote: >> On 12/10/2015 12:44 PM, Tom Evans wrote: >>> I've just had to delve back into the deep murky past and try to get >>> Freescale's Kernel 2.6.35 FlexCAN driver working... > > On an i.MX53. That's the last supported kernel Freescale ever released for > that series of parts. > The simplest path was to fix the > simple and stupidly obvious bugs in the existing driver rather > than add new complicated bugs. There seems to be no end of bugs in this driver. It is a lesson in something. What's the dumbest driver bug ever? Interrupt hazard. Yes, this one has that too. The mainline sends a Message Buffer (or tries to) and DISABLES the netif queue if it can't send [1]. The transmit interrupt service routine ENABLES the netif queue. And when the interrupt routine happens in the middle of the mainline transmit routine? It locks forever. You have to power-cycle to get it back. Normally taking the interface "down" and "up" should fix it, as the "start" and "stop" functions normally stop and start the netif queue. Not these ones. They do now. Note 1: This is in the case where the transmit queues and MBs are full. The transmit function is always called twice. The first time it is called it sends the MB, leaves the queue running and return "OK". The second time it finds out it is full, stops the queue and returns "BUSY". Even better, with the default 32 transmit MBs it has to scan through all 32 MBs to find out it is full, and it may have also had to scan the same 32 to find an empty one. The FlexCAN registers are on a slow bus and take 180ns to read or write. So it can take the driver up to 12us to send one buffer. Or proportionally longer if you have more Transmit MBs (you might have 56). If you never send enough data to flow-control the driver this won't happen. If you have more than one Transmit MB (and can handle out of order transmissions) then the subsequent transmit interrupts will repair the damage and you won't see it either. That's probably why it passed whatever testing it had. Bug detailed here: https://community.freescale.com/message/597952 Patch to fix here: https://community.freescale.com/message/597951#597951 Tom ^ permalink raw reply [flat|nested] 41+ messages in thread
* can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works. @ 2015-12-17 4:22 ` Tom Evans 0 siblings, 0 replies; 41+ messages in thread From: Tom Evans @ 2015-12-17 4:22 UTC (permalink / raw) To: linux-arm-kernel On 11/12/15 09:53, Tom Evans wrote: > On 10/12/2015 11:44 PM, Marc Kleine-Budde wrote: >> On 12/10/2015 12:44 PM, Tom Evans wrote: >>> I've just had to delve back into the deep murky past and try to get >>> Freescale's Kernel 2.6.35 FlexCAN driver working... > > On an i.MX53. That's the last supported kernel Freescale ever released for > that series of parts. > The simplest path was to fix the > simple and stupidly obvious bugs in the existing driver rather > than add new complicated bugs. There seems to be no end of bugs in this driver. It is a lesson in something. What's the dumbest driver bug ever? Interrupt hazard. Yes, this one has that too. The mainline sends a Message Buffer (or tries to) and DISABLES the netif queue if it can't send [1]. The transmit interrupt service routine ENABLES the netif queue. And when the interrupt routine happens in the middle of the mainline transmit routine? It locks forever. You have to power-cycle to get it back. Normally taking the interface "down" and "up" should fix it, as the "start" and "stop" functions normally stop and start the netif queue. Not these ones. They do now. Note 1: This is in the case where the transmit queues and MBs are full. The transmit function is always called twice. The first time it is called it sends the MB, leaves the queue running and return "OK". The second time it finds out it is full, stops the queue and returns "BUSY". Even better, with the default 32 transmit MBs it has to scan through all 32 MBs to find out it is full, and it may have also had to scan the same 32 to find an empty one. The FlexCAN registers are on a slow bus and take 180ns to read or write. So it can take the driver up to 12us to send one buffer. Or proportionally longer if you have more Transmit MBs (you might have 56). If you never send enough data to flow-control the driver this won't happen. If you have more than one Transmit MB (and can handle out of order transmissions) then the subsequent transmit interrupts will repair the damage and you won't see it either. That's probably why it passed whatever testing it had. Bug detailed here: https://community.freescale.com/message/597952 Patch to fix here: https://community.freescale.com/message/597951#597951 Tom ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works. 2015-12-17 4:22 ` Tom Evans @ 2015-12-23 0:53 ` Tom Evans -1 siblings, 0 replies; 41+ messages in thread From: Tom Evans @ 2015-12-23 0:53 UTC (permalink / raw) To: arnd, linux-can; +Cc: linux-arm-kernel On 17/12/15 15:22, Tom Evans wrote: > On 11/12/15 09:53, Tom Evans wrote: >> On 10/12/2015 11:44 PM, Marc Kleine-Budde wrote: >> >> The simplest path was to fix the simple and stupidly obvious >> bugs in the existing driver rather than add new complicated bugs. > > There seems to be no end of bugs in this driver. It is a lesson > in something. The Bus Off Recovery doesn't work either. A Bus Off condition results in the module being soft-reset and then never woken up again, mainly because the calls to "mod_timer()" forget to add "jiffies", so the timer doesn't trigger. Then the Auto/Manual mode recovery test is probably backwards. Then it tests MDIS instead of HALT, so it never resets the module after the soft reset. If you program it in "Auto Recovery" mode it resets it anyway. Bugs detailed here: https://community.freescale.com/message/599099#599099 There'll probably be a link on that page to the patches to fix it after I've tested it some more. Tom ^ permalink raw reply [flat|nested] 41+ messages in thread
* can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works. @ 2015-12-23 0:53 ` Tom Evans 0 siblings, 0 replies; 41+ messages in thread From: Tom Evans @ 2015-12-23 0:53 UTC (permalink / raw) To: linux-arm-kernel On 17/12/15 15:22, Tom Evans wrote: > On 11/12/15 09:53, Tom Evans wrote: >> On 10/12/2015 11:44 PM, Marc Kleine-Budde wrote: >> >> The simplest path was to fix the simple and stupidly obvious >> bugs in the existing driver rather than add new complicated bugs. > > There seems to be no end of bugs in this driver. It is a lesson > in something. The Bus Off Recovery doesn't work either. A Bus Off condition results in the module being soft-reset and then never woken up again, mainly because the calls to "mod_timer()" forget to add "jiffies", so the timer doesn't trigger. Then the Auto/Manual mode recovery test is probably backwards. Then it tests MDIS instead of HALT, so it never resets the module after the soft reset. If you program it in "Auto Recovery" mode it resets it anyway. Bugs detailed here: https://community.freescale.com/message/599099#599099 There'll probably be a link on that page to the patches to fix it after I've tested it some more. Tom ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode 2015-12-10 11:05 ` Sharma Bhupesh @ 2015-12-10 12:19 ` Marc Kleine-Budde -1 siblings, 0 replies; 41+ messages in thread From: Marc Kleine-Budde @ 2015-12-10 12:19 UTC (permalink / raw) To: Sharma Bhupesh, arnd, linux-can Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar [-- Attachment #1: Type: text/plain, Size: 2685 bytes --] On 12/10/2015 12:05 PM, Sharma Bhupesh wrote: >>> -----Original Message----- >>> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de] >>> Sent: Thursday, May 14, 2015 9:12 PM >>> To: Sharma Bhupesh-B45370; arnd@arndb.de; linux-can@vger.kernel.org >>> Cc: bhupesh.linux@gmail.com; Arora Sakar-B45205; linux-arm- >>> kernel@lists.infradead.org >>> Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO >>> mode >>> >>> On 05/14/2015 01:33 PM, Bhupesh Sharma wrote: >>>> This patch adds support for non RX-FIFO (legacy) mode in the flexcan >>>> driver. >>>> >>>> On certain SoCs, the RX-FIFO support might be broken, as a result we >>>> need to fall-back on the legacy (non RX-FIFO) mode to receive CAN >>>> frames. >>>> >>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> >>>> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> >>> >>> The non FIFO mode doesn't guarantee the order of the incoming frames, >>> not does not even try to...this is not acceptable. I'm currently >>> working on a patch by David Jander that brings in non FIFO mode, but >>> tries to keep the order of the frames. >> >> That is already WIP at our end. V3 will contain the same change. >> If you are already working on it, I don't know how to proceed further as >> we had already v1 of this patchset with the non FIFO mode out since a >> month or so. > > I don't remember seeing a patch from you which supports non-FIFO mode for flexcan IP. > Since we now have Freescale LS1021A chip out in the field on which the FIFO mode broken, > we cannot use the upstream flexcan driver for enabling flexcan IP on these chips. > > Do you still have plans to work on this? Or should I submit my v3 with in-order reception > supported for rx frames. The problem is, that the current code triggers a ordering issue (at least on the imx6) in non FIFO mode. The frames are not received as the documentation states. If you have access, take a look at "SR# 1-4074792564 : CAN Ordering Issues". It seems the freescale support doesn't have much interest in confirming the issue, nor bringing us in touch with the people who have access to the IP core source to see what's going on. If we have to rely on the timestamps the next logical step is to add sorting by timestamp to the rx-fifo implementation. I'll send a patch series of the current state. regards, 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: 455 bytes --] ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode @ 2015-12-10 12:19 ` Marc Kleine-Budde 0 siblings, 0 replies; 41+ messages in thread From: Marc Kleine-Budde @ 2015-12-10 12:19 UTC (permalink / raw) To: linux-arm-kernel On 12/10/2015 12:05 PM, Sharma Bhupesh wrote: >>> -----Original Message----- >>> From: Marc Kleine-Budde [mailto:mkl at pengutronix.de] >>> Sent: Thursday, May 14, 2015 9:12 PM >>> To: Sharma Bhupesh-B45370; arnd at arndb.de; linux-can at vger.kernel.org >>> Cc: bhupesh.linux at gmail.com; Arora Sakar-B45205; linux-arm- >>> kernel at lists.infradead.org >>> Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO >>> mode >>> >>> On 05/14/2015 01:33 PM, Bhupesh Sharma wrote: >>>> This patch adds support for non RX-FIFO (legacy) mode in the flexcan >>>> driver. >>>> >>>> On certain SoCs, the RX-FIFO support might be broken, as a result we >>>> need to fall-back on the legacy (non RX-FIFO) mode to receive CAN >>>> frames. >>>> >>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> >>>> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> >>> >>> The non FIFO mode doesn't guarantee the order of the incoming frames, >>> not does not even try to...this is not acceptable. I'm currently >>> working on a patch by David Jander that brings in non FIFO mode, but >>> tries to keep the order of the frames. >> >> That is already WIP at our end. V3 will contain the same change. >> If you are already working on it, I don't know how to proceed further as >> we had already v1 of this patchset with the non FIFO mode out since a >> month or so. > > I don't remember seeing a patch from you which supports non-FIFO mode for flexcan IP. > Since we now have Freescale LS1021A chip out in the field on which the FIFO mode broken, > we cannot use the upstream flexcan driver for enabling flexcan IP on these chips. > > Do you still have plans to work on this? Or should I submit my v3 with in-order reception > supported for rx frames. The problem is, that the current code triggers a ordering issue (at least on the imx6) in non FIFO mode. The frames are not received as the documentation states. If you have access, take a look at "SR# 1-4074792564 : CAN Ordering Issues". It seems the freescale support doesn't have much interest in confirming the issue, nor bringing us in touch with the people who have access to the IP core source to see what's going on. If we have to rely on the timestamps the next logical step is to add sorting by timestamp to the rx-fifo implementation. I'll send a patch series of the current state. regards, 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 | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 455 bytes Desc: OpenPGP digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151210/496f3ff5/attachment.sig> ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode 2015-12-10 12:19 ` Marc Kleine-Budde @ 2015-12-10 12:22 ` Sharma Bhupesh -1 siblings, 0 replies; 41+ messages in thread From: Sharma Bhupesh @ 2015-12-10 12:22 UTC (permalink / raw) To: Marc Kleine-Budde, arnd, linux-can Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar > -----Original Message----- > From: Marc Kleine-Budde [mailto:mkl@pengutronix.de] > Sent: Thursday, December 10, 2015 5:49 PM > > On 12/10/2015 12:05 PM, Sharma Bhupesh wrote: > >>> -----Original Message----- > >>> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de] > >>> Sent: Thursday, May 14, 2015 9:12 PM > >>> To: Sharma Bhupesh-B45370; arnd@arndb.de; linux-can@vger.kernel.org > >>> Cc: bhupesh.linux@gmail.com; Arora Sakar-B45205; linux-arm- > >>> kernel@lists.infradead.org > >>> Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non > >>> RX-FIFO mode > >>> > >>> On 05/14/2015 01:33 PM, Bhupesh Sharma wrote: > >>>> This patch adds support for non RX-FIFO (legacy) mode in the > >>>> flexcan driver. > >>>> > >>>> On certain SoCs, the RX-FIFO support might be broken, as a result > >>>> we need to fall-back on the legacy (non RX-FIFO) mode to receive > >>>> CAN frames. > >>>> > >>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> > >>>> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> > >>> > >>> The non FIFO mode doesn't guarantee the order of the incoming > >>> frames, not does not even try to...this is not acceptable. I'm > >>> currently working on a patch by David Jander that brings in non FIFO > >>> mode, but tries to keep the order of the frames. > >> > >> That is already WIP at our end. V3 will contain the same change. > >> If you are already working on it, I don't know how to proceed further > >> as we had already v1 of this patchset with the non FIFO mode out > >> since a month or so. > > > > I don't remember seeing a patch from you which supports non-FIFO mode > for flexcan IP. > > Since we now have Freescale LS1021A chip out in the field on which the > > FIFO mode broken, we cannot use the upstream flexcan driver for > enabling flexcan IP on these chips. > > > > Do you still have plans to work on this? Or should I submit my v3 with > > in-order reception supported for rx frames. > > The problem is, that the current code triggers a ordering issue (at least > on the imx6) in non FIFO mode. The frames are not received as the > documentation states. If you have access, take a look at "SR# > 1-4074792564 : CAN Ordering Issues". It seems the freescale support > doesn't have much interest in confirming the issue, nor bringing us in > touch with the people who have access to the IP core source to see what's > going on. > > If we have to rely on the timestamps the next logical step is to add > sorting by timestamp to the rx-fifo implementation. I'll send a patch > series of the current state. With my current v3, I see no rx-frame ordering issues atleast on LS1021A. I can ask internally about more details on the "SR# 1-4074792564 : CAN Ordering Issues". Regards, Bhupesh ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode @ 2015-12-10 12:22 ` Sharma Bhupesh 0 siblings, 0 replies; 41+ messages in thread From: Sharma Bhupesh @ 2015-12-10 12:22 UTC (permalink / raw) To: linux-arm-kernel > -----Original Message----- > From: Marc Kleine-Budde [mailto:mkl at pengutronix.de] > Sent: Thursday, December 10, 2015 5:49 PM > > On 12/10/2015 12:05 PM, Sharma Bhupesh wrote: > >>> -----Original Message----- > >>> From: Marc Kleine-Budde [mailto:mkl at pengutronix.de] > >>> Sent: Thursday, May 14, 2015 9:12 PM > >>> To: Sharma Bhupesh-B45370; arnd at arndb.de; linux-can at vger.kernel.org > >>> Cc: bhupesh.linux at gmail.com; Arora Sakar-B45205; linux-arm- > >>> kernel at lists.infradead.org > >>> Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non > >>> RX-FIFO mode > >>> > >>> On 05/14/2015 01:33 PM, Bhupesh Sharma wrote: > >>>> This patch adds support for non RX-FIFO (legacy) mode in the > >>>> flexcan driver. > >>>> > >>>> On certain SoCs, the RX-FIFO support might be broken, as a result > >>>> we need to fall-back on the legacy (non RX-FIFO) mode to receive > >>>> CAN frames. > >>>> > >>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> > >>>> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> > >>> > >>> The non FIFO mode doesn't guarantee the order of the incoming > >>> frames, not does not even try to...this is not acceptable. I'm > >>> currently working on a patch by David Jander that brings in non FIFO > >>> mode, but tries to keep the order of the frames. > >> > >> That is already WIP at our end. V3 will contain the same change. > >> If you are already working on it, I don't know how to proceed further > >> as we had already v1 of this patchset with the non FIFO mode out > >> since a month or so. > > > > I don't remember seeing a patch from you which supports non-FIFO mode > for flexcan IP. > > Since we now have Freescale LS1021A chip out in the field on which the > > FIFO mode broken, we cannot use the upstream flexcan driver for > enabling flexcan IP on these chips. > > > > Do you still have plans to work on this? Or should I submit my v3 with > > in-order reception supported for rx frames. > > The problem is, that the current code triggers a ordering issue (at least > on the imx6) in non FIFO mode. The frames are not received as the > documentation states. If you have access, take a look at "SR# > 1-4074792564 : CAN Ordering Issues". It seems the freescale support > doesn't have much interest in confirming the issue, nor bringing us in > touch with the people who have access to the IP core source to see what's > going on. > > If we have to rely on the timestamps the next logical step is to add > sorting by timestamp to the rx-fifo implementation. I'll send a patch > series of the current state. With my current v3, I see no rx-frame ordering issues atleast on LS1021A. I can ask internally about more details on the "SR# 1-4074792564 : CAN Ordering Issues". Regards, Bhupesh ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode 2015-12-10 12:22 ` Sharma Bhupesh @ 2015-12-11 16:01 ` Robert Schwebel -1 siblings, 0 replies; 41+ messages in thread From: Robert Schwebel @ 2015-12-11 16:01 UTC (permalink / raw) To: Sharma Bhupesh Cc: Marc Kleine-Budde, arnd, linux-can, bhupesh.linux, linux-arm-kernel, Arora Sakar On Thu, Dec 10, 2015 at 12:22:55PM +0000, Sharma Bhupesh wrote: > > If we have to rely on the timestamps the next logical step is to add > > sorting by timestamp to the rx-fifo implementation. I'll send a > > patch series of the current state. > > With my current v3, I see no rx-frame ordering issues atleast on > LS1021A. Marc can post our testcases here on the list (maybe with the description from the service request); perhaps this can clarify the issues. > I can ask internally about more details on the "SR# 1-4074792564 : CAN > Ordering Issues". That would be quite helpful, thanks! Understanding the root cause of the issue is often a good start, maybe there is an option for a software workaround. rsc -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode @ 2015-12-11 16:01 ` Robert Schwebel 0 siblings, 0 replies; 41+ messages in thread From: Robert Schwebel @ 2015-12-11 16:01 UTC (permalink / raw) To: linux-arm-kernel On Thu, Dec 10, 2015 at 12:22:55PM +0000, Sharma Bhupesh wrote: > > If we have to rely on the timestamps the next logical step is to add > > sorting by timestamp to the rx-fifo implementation. I'll send a > > patch series of the current state. > > With my current v3, I see no rx-frame ordering issues atleast on > LS1021A. Marc can post our testcases here on the list (maybe with the description from the service request); perhaps this can clarify the issues. > I can ask internally about more details on the "SR# 1-4074792564 : CAN > Ordering Issues". That would be quite helpful, thanks! Understanding the root cause of the issue is often a good start, maybe there is an option for a software workaround. rsc -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode 2015-05-14 15:41 ` Marc Kleine-Budde @ 2015-05-15 0:09 ` Tom Evans -1 siblings, 0 replies; 41+ messages in thread From: Tom Evans @ 2015-05-15 0:09 UTC (permalink / raw) To: Marc Kleine-Budde, Bhupesh Sharma, arnd, linux-can Cc: bhupesh.linux, Sakar.Arora, linux-arm-kernel On 15/05/15 01:41, Marc Kleine-Budde wrote: > On 05/14/2015 01:33 PM, Bhupesh Sharma wrote: >> This patch adds support for non RX-FIFO (legacy) mode in >> the flexcan driver. >> >> On certain SoCs, the RX-FIFO support might be broken, as >> a result we need to fall-back on the legacy (non RX-FIFO) >> mode to receive CAN frames. >> >> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> >> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> > > The non FIFO mode doesn't guarantee the order of the incoming frames, > not does not even try to...this is not acceptable. I'm currently working > on a patch by David Jander that brings in non FIFO mode, but tries to > keep the order of the frames. > > Marc In case it is of any help as a reference, here's a thread on the problems of receiving messages in order on another non-FIFO chip. This applies to the normal Microchip CAN controllers, in this case the very-hard-to-program on-the-end-of-an-SPI-bus MCP2515: http://www.microchip.com/forums/tm.aspx?m=620741 It has two receive buffers, and a "rollover" setting. This does not make the registers operate as a FIFO and it needs a state machine to keep track of which buffer holds which sequence message. Tom ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode @ 2015-05-15 0:09 ` Tom Evans 0 siblings, 0 replies; 41+ messages in thread From: Tom Evans @ 2015-05-15 0:09 UTC (permalink / raw) To: linux-arm-kernel On 15/05/15 01:41, Marc Kleine-Budde wrote: > On 05/14/2015 01:33 PM, Bhupesh Sharma wrote: >> This patch adds support for non RX-FIFO (legacy) mode in >> the flexcan driver. >> >> On certain SoCs, the RX-FIFO support might be broken, as >> a result we need to fall-back on the legacy (non RX-FIFO) >> mode to receive CAN frames. >> >> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com> >> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com> > > The non FIFO mode doesn't guarantee the order of the incoming frames, > not does not even try to...this is not acceptable. I'm currently working > on a patch by David Jander that brings in non FIFO mode, but tries to > keep the order of the frames. > > Marc In case it is of any help as a reference, here's a thread on the problems of receiving messages in order on another non-FIFO chip. This applies to the normal Microchip CAN controllers, in this case the very-hard-to-program on-the-end-of-an-SPI-bus MCP2515: http://www.microchip.com/forums/tm.aspx?m=620741 It has two receive buffers, and a "rollover" setting. This does not make the registers operate as a FIFO and it needs a state machine to keep track of which buffer holds which sequence message. Tom ^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2015-12-23 0:53 UTC | newest] Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-05-14 11:33 [PATCH v2 0/5] Add flexcan support for LS1021A SoCs Bhupesh Sharma 2015-05-14 11:33 ` Bhupesh Sharma 2015-05-14 11:33 ` [PATCH v2 1/5] doc/bindings: Add 'endianess' optional-property for FlexCAN controller Bhupesh Sharma 2015-05-14 11:33 ` Bhupesh Sharma 2015-05-14 11:33 ` [PATCH v2 2/5] arm/dts: Add nodes for flexcan devices present on LS1021A SoC Bhupesh Sharma 2015-05-14 11:33 ` Bhupesh Sharma 2015-05-14 11:33 ` [PATCH v2 3/5] can: flexcan: Add ls1021a flexcan device entry Bhupesh Sharma 2015-05-14 11:33 ` Bhupesh Sharma 2015-05-14 15:38 ` Marc Kleine-Budde 2015-05-14 15:38 ` Marc Kleine-Budde 2015-05-14 11:33 ` [PATCH v2 4/5] can: flexcan: Remodel FlexCAN register r/w APIs for BE instances Bhupesh Sharma 2015-05-14 11:33 ` Bhupesh Sharma 2015-05-18 16:17 ` Enrico Weigelt, metux IT consult 2015-05-18 16:17 ` Enrico Weigelt, metux IT consult 2015-05-18 16:37 ` Sharma Bhupesh 2015-05-18 16:37 ` Sharma Bhupesh 2015-05-14 11:33 ` [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode Bhupesh Sharma 2015-05-14 11:33 ` Bhupesh Sharma 2015-05-14 15:41 ` Marc Kleine-Budde 2015-05-14 15:41 ` Marc Kleine-Budde 2015-05-14 15:44 ` Sharma Bhupesh 2015-05-14 15:44 ` Sharma Bhupesh 2015-12-10 11:05 ` Sharma Bhupesh 2015-12-10 11:05 ` Sharma Bhupesh 2015-12-10 11:44 ` can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works Tom Evans 2015-12-10 12:44 ` Marc Kleine-Budde 2015-12-10 12:44 ` Marc Kleine-Budde 2015-12-10 22:53 ` Tom Evans 2015-12-10 22:53 ` Tom Evans 2015-12-17 4:22 ` Tom Evans 2015-12-17 4:22 ` Tom Evans 2015-12-23 0:53 ` Tom Evans 2015-12-23 0:53 ` Tom Evans 2015-12-10 12:19 ` [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode Marc Kleine-Budde 2015-12-10 12:19 ` Marc Kleine-Budde 2015-12-10 12:22 ` Sharma Bhupesh 2015-12-10 12:22 ` Sharma Bhupesh 2015-12-11 16:01 ` Robert Schwebel 2015-12-11 16:01 ` Robert Schwebel 2015-05-15 0:09 ` Tom Evans 2015-05-15 0:09 ` Tom Evans
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.