RE: [1/4] Device tree entry for Freescale TDM controller
-Original Message- From: Wood Scott-B07421 Sent: 29 June 2013 03:46 To: Singh Sandeep-B37400 Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-ker...@lists.infradead.org; Aggrwal Poonam-B10812 Subject: Re: [1/4] Device tree entry for Freescale TDM controller On Thu, Mar 07, 2013 at 04:57:45PM +0530, Sandeep Singh wrote: +tdm@16000 { + compatible = fsl,tdm1.0; + reg = 0x16000 0x200 0x2c000 0x2000; + clock-frequency = 0; + tdm_tx_clk = 2048000; + interrupts = 62 8 0 0; + fsl,max-time-slots = 128; +}; tdm_tx_clk isn't in the binding (and should be named fsl,tdm-tx-clk if it is meant to be here at all). I will add it in the binding. Thanks, Sandeep -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/4] Device tree entry for Freescale TDM controller
A gentle reminder. Any comments are appreciated. Regards, Sandeep -Original Message- From: Linuxppc-dev [mailto:linuxppc-dev- bounces+sandeep=freescale@lists.ozlabs.org] On Behalf Of Singh Sandeep-B37400 Sent: Wednesday, March 20, 2013 4:22 PM To: linuxppc-dev@lists.ozlabs.org; linux-arm-ker...@lists.infradead.org Cc: Aggrwal Poonam-B10812 Subject: RE: [PATCH 1/4] Device tree entry for Freescale TDM controller Any comments on this patch set?? Regards, Sandeep -Original Message- From: Singh Sandeep-B37400 Sent: Thursday, March 07, 2013 4:58 PM To: linuxppc-dev@lists.ozlabs.org; linux-arm-ker...@lists.infradead.org Cc: Singh Sandeep-B37400; Aggrwal Poonam-B10812 Subject: [PATCH 1/4] Device tree entry for Freescale TDM controller ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/4] Device tree entry for Freescale TDM controller
Any comments on this patch set?? Regards, Sandeep -Original Message- From: Singh Sandeep-B37400 Sent: Thursday, March 07, 2013 4:58 PM To: linuxppc-dev@lists.ozlabs.org; linux-arm-ker...@lists.infradead.org Cc: Singh Sandeep-B37400; Aggrwal Poonam-B10812 Subject: [PATCH 1/4] Device tree entry for Freescale TDM controller ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/4] Device tree entry for Freescale TDM controller
-Original Message- From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk] Sent: Thursday, March 07, 2013 7:04 PM To: Singh Sandeep-B37400 Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-ker...@lists.infradead.org; Aggrwal Poonam-B10812 Subject: Re: [PATCH 1/4] Device tree entry for Freescale TDM controller I know I'm probably missing something... but... why are these patches copied to the ARM list? They appear to be PowerPC patches. There was a request from few ARM Linux developers that they were interested in developing TDM subsystem for ARM and would like to see TDM patches in ARM mailing list. Just trying to oblige On Thu, Mar 07, 2013 at 04:57:45PM +0530, Sandeep Singh wrote: Added dtsi file for Freescale TDM controller. This controller is available on many Freescale SOCs like MPC8315, P1020, P1010, P1022 and P1024 Signed-off-by: Sandeep Singh sand...@freescale.com Signed-off-by: Poonam Aggrwal poonam.aggr...@freescale.com --- arch/powerpc/boot/dts/fsl/pq3-tdm1.0-0.dtsi | 42 +++ 1 files changed, 42 insertions(+), 0 deletions(-) create mode 100644 arch/powerpc/boot/dts/fsl/pq3-tdm1.0-0.dtsi diff --git a/arch/powerpc/boot/dts/fsl/pq3-tdm1.0-0.dtsi b/arch/powerpc/boot/dts/fsl/pq3-tdm1.0-0.dtsi new file mode 100644 index 000..e89f637 --- /dev/null +++ b/arch/powerpc/boot/dts/fsl/pq3-tdm1.0-0.dtsi @@ -0,0 +1,42 @@ +/* + * PQ3 TDM device tree stub [ controller @ offset 0x16000 ] + * + * Copyright 2012 Freescale Semiconductor Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of Freescale Semiconductor nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * + * ALTERNATIVELY, this software may be distributed under the terms of +the + * GNU General Public License (GPL) as published by the Free +Software + * Foundation, either version 2 of that License or (at your option) +any + * later version. + * + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND +ANY + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +ARE + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE +FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE +USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +tdm@16000 { + compatible = fsl,tdm1.0; + reg = 0x16000 0x200 0x2c000 0x2000; + clock-frequency = 0; + tdm_tx_clk = 2048000; + interrupts = 62 8 0 0; + fsl,max-time-slots = 128; +}; -- 1.7.6.GIT ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/4] TDM Device Tree entries for various Freescale Platforms
-Original Message- From: Timur Tabi [mailto:ti...@tabi.org] Sent: Friday, March 08, 2013 10:17 PM To: Singh Sandeep-B37400 Cc: linuxppc-dev@lists.ozlabs.org; Aggrwal Poonam-B10812 Subject: Re: [PATCH 3/4] TDM Device Tree entries for various Freescale Platforms On Thu, Mar 7, 2013 at 5:27 AM, Sandeep Singh sand...@freescale.com wrote: arch/powerpc/boot/dts/p1022ds_36b.dts |3 +++ You forgot to update p1022ds_3b.dts. p1022ds_32b.dts includes fsl/p1022si-post.dtsi which is getting updated. - * Copyright 2011 Freescale Semiconductor Inc. + * Copyright 2011-2012 Freescale Semiconductor Inc. Do not update the copyright year in patches. Ok, will correct this. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] Added device tree binding for TDM and TDM phy
-Original Message- From: Wood Scott-B07421 Sent: Thursday, January 10, 2013 5:37 AM To: Singh Sandeep-B37400 Cc: devicetree-disc...@lists.ozlabs.org; linuxppc-...@ozlabs.org; Aggrwal Poonam-B10812 Subject: Re: [PATCH] Added device tree binding for TDM and TDM phy On 01/09/2013 01:10:24 AM, Singh Sandeep-B37400 wrote: A gentle reminder. Any comments are appreciated. Regards, Sandeep -Original Message- From: Singh Sandeep-B37400 Sent: Wednesday, January 02, 2013 6:55 PM To: devicetree-disc...@lists.ozlabs.org; linuxppc-...@ozlabs.org Cc: Singh Sandeep-B37400; Aggrwal Poonam-B10812 Subject: [PATCH] Added device tree binding for TDM and TDM phy This controller is available on many Freescale SOCs like MPC8315, P1020, P1010 and P1022 Signed-off-by: Sandeep Singh sand...@freescale.com Signed-off-by: Poonam Aggrwal poonam.aggr...@freescale.com --- .../devicetree/bindings/powerpc/fsl/fsl-tdm.txt| 63 .../devicetree/bindings/powerpc/fsl/tdm-phy.txt| 38 2 files changed, 101 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/tdm- phy.txt diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt b/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt new file mode 100644 index 000..ceb2ef1 --- /dev/null +++ b/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt @@ -0,0 +1,63 @@ +TDM Device Tree Binding + +NOTE: The bindings described in this document are preliminary and +subject to change. + +TDM (Time Division Multiplexing) + +Description: + +The TDM is full duplex serial port designed to allow various devices +including digital signal processors (DSPs) to communicate with a +variety of serial devices including industry standard framers, codecs, other DSPs and microprocessors. + +The below properties describe the device tree bindings for Freescale +TDM controller. This TDM controller is available on various Freescale +Processors like MPC8315, P1020, P1022 and P1010. + +Required properties: + +- compatible +Value type: string +Definition: Should contain fsl,tdm1.0. + +- reg +Definition: A standard property. The first reg specifier describes the TDM +registers, and the second describes the TDM DMAC registers. + +- tdm_tx_clk +Value type: u32 or u64 +Definition: This specifies the value of transmit clock. It should not +exceed 50Mhz. + +- tdm_rx_clk +Value type: u32 or u64 +Definition: This specifies the value of receive clock. Its value could be +zero, in which case tdm will operate in shared mode. Its value should not +exceed 50Mhz. Please don't use underscores in property names, and use the vendor prefix: fsl,tdm-tx-clk and fsl,tdm-rx-clk. Ok. diff --git a/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt b/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt new file mode 100644 index 000..2563934 --- /dev/null +++ b/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt @@ -0,0 +1,38 @@ +TDM PHY Device Tree Binding + +NOTE: The bindings described in this document are preliminary and +subject to change. + +Description: +TDM PHY is the terminal interface of TDM subsystem. It is typically a +line control device like E1/T1 framer or SLIC. A TDM device can have +multiple TDM PHYs. + +Required properties: + +- compatible +Value type: string +Definition: Should contain generic compatibility like tdm-phy-slic or +tdm-phy-e1 or tdm-phy-t1. Does this generic string (plus the other properties) tell you all you need to know about the device? If there are other possible generic compatibles, they should be listed or else different people will make up different strings for the same thing. This property will describe the type of device, and will help TDM framework to know if it is E1/T1/SLIC device. Further details can be extracted from other compatible strings. There are only three generic compatibles field types, which are already mentioned in definition. Do I need to make this thing more clear. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] Added device tree binding for TDM and TDM phy
-Original Message- From: Wood Scott-B07421 Sent: Friday, January 11, 2013 1:19 AM To: Singh Sandeep-B37400 Cc: Wood Scott-B07421; devicetree-disc...@lists.ozlabs.org; linuxppc- d...@ozlabs.org; Aggrwal Poonam-B10812 Subject: Re: [PATCH] Added device tree binding for TDM and TDM phy On 01/10/2013 03:24:21 AM, Singh Sandeep-B37400 wrote: +- compatible +Value type: string +Definition: Should contain generic compatibility like tdm-phy-slic or +tdm-phy-e1 or tdm-phy-t1. Does this generic string (plus the other properties) tell you all you need to know about the device? If there are other possible generic compatibles, they should be listed or else different people will make up different strings for the same thing. This property will describe the type of device, and will help TDM framework to know if it is E1/T1/SLIC device. Further details can be extracted from other compatible strings. There are only three generic compatibles field types, which are already mentioned in definition. Do I need to make this thing more clear. The word like suggests that there are other possibilites. It would be clearer as: Definition: One of tdm-phy-slic, tdm-phy-e1, or tdm-phy-t1. -Scott Ok, thanks for your comments. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] Added device tree binding for TDM and TDM phy
A gentle reminder. Any comments are appreciated. Regards, Sandeep -Original Message- From: Singh Sandeep-B37400 Sent: Wednesday, January 02, 2013 6:55 PM To: devicetree-disc...@lists.ozlabs.org; linuxppc-...@ozlabs.org Cc: Singh Sandeep-B37400; Aggrwal Poonam-B10812 Subject: [PATCH] Added device tree binding for TDM and TDM phy This controller is available on many Freescale SOCs like MPC8315, P1020, P1010 and P1022 Signed-off-by: Sandeep Singh sand...@freescale.com Signed-off-by: Poonam Aggrwal poonam.aggr...@freescale.com --- .../devicetree/bindings/powerpc/fsl/fsl-tdm.txt| 63 .../devicetree/bindings/powerpc/fsl/tdm-phy.txt| 38 2 files changed, 101 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/tdm- phy.txt diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt b/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt new file mode 100644 index 000..ceb2ef1 --- /dev/null +++ b/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt @@ -0,0 +1,63 @@ +TDM Device Tree Binding + +NOTE: The bindings described in this document are preliminary and +subject to change. + +TDM (Time Division Multiplexing) + +Description: + +The TDM is full duplex serial port designed to allow various devices +including digital signal processors (DSPs) to communicate with a +variety of serial devices including industry standard framers, codecs, other DSPs and microprocessors. + +The below properties describe the device tree bindings for Freescale +TDM controller. This TDM controller is available on various Freescale +Processors like MPC8315, P1020, P1022 and P1010. + +Required properties: + +- compatible +Value type: string +Definition: Should contain fsl,tdm1.0. + +- reg +Definition: A standard property. The first reg specifier describes the TDM +registers, and the second describes the TDM DMAC registers. + +- tdm_tx_clk +Value type: u32 or u64 +Definition: This specifies the value of transmit clock. It should not +exceed 50Mhz. + +- tdm_rx_clk +Value type: u32 or u64 +Definition: This specifies the value of receive clock. Its value could be +zero, in which case tdm will operate in shared mode. Its value should not +exceed 50Mhz. + +- interrupts +Definition: Two interrupt specifiers. The first is TDM error, and the +second is TDM DMAC. + +- phy-handle +Value type: phandle +Definition: Phandle of the line controller node or framer node eg. SLIC, +E1/T1 etc. (Refer +Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt) + +- fsl,max-time-slots +Value type: u32 +Definition: Maximum number of 8-bit time slots in one TDM frame. This is +the maximum number which TDM hardware supports. + +Example: + + tdm@16000 { + compatible = fsl,tdm1.0; + reg = 0x16000 0x200 0x2c000 0x2000; + tdm_tx_clk = 2048000; + tdm_rx_clk = 0; + interrupts = 16 8 62 8; + phy-handle = tdm-phy; + fsl,max-time-slots = 128; + }; diff --git a/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt b/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt new file mode 100644 index 000..2563934 --- /dev/null +++ b/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt @@ -0,0 +1,38 @@ +TDM PHY Device Tree Binding + +NOTE: The bindings described in this document are preliminary and +subject to change. + +Description: +TDM PHY is the terminal interface of TDM subsystem. It is typically a +line control device like E1/T1 framer or SLIC. A TDM device can have +multiple TDM PHYs. + +Required properties: + +- compatible +Value type: string +Definition: Should contain generic compatibility like tdm-phy-slic or +tdm-phy-e1 or tdm-phy-t1. + +- max-num-ports +Value type: u32 +Definition: Defines the maximum number of ports supported by the SLIC +device. Only required if the device is SLIC. For E1/T1 devices the number +of ports are predefined i.e. (24 in case of T1 and 32 in case of E1). + +Apart from the above, there may be other properties required because of +the bus/interface this device is connected on. It could be SPI/local bus, etc. + +Example: + + tdm-phy@0 { + compatible = zarlink,le88266,tdm-phy-slic; + reg = 0; + max-num-ports = 4; + spi-max-frequency = 800; + }; + +In the above example properties reg and spi-max-frequency are SPI +specific as the SLIC device is connected on SPI interface. These +properties might vary depending on the specific interface the device is using. -- 1.7.6.GIT ___ Linuxppc
RE: [2/3][PATCH][v2] TDM Framework
-Original Message- From: Greg KH [mailto:g...@kroah.com] Sent: Monday, July 30, 2012 9:32 PM To: Singh Sandeep-B37400 Cc: Francois Romieu; de...@driverdev.osuosl.org; linuxppc- d...@lists.ozlabs.org; ga...@kernel.crashing.org; linux-arm- ker...@lists.infradead.org; linux-ker...@vger.kernel.org Subject: Re: [2/3][PATCH][v2] TDM Framework On Mon, Jul 30, 2012 at 09:50:57AM +, Singh Sandeep-B37400 wrote: 1. You should send some kernel mode TDM clients. Without those the framework is pretty useless. [Sandeep] We do have a test client but not good enough to be pushed in open source, should we add it to documentation?? Then how do you know if the framework is correct and good enough for real clients? We don't add frameworks, or apis, to the kernel without users, so you will have to come up with some users before we can accept it. We can only say that this framework is available in FSL BSPs and being used by VoIP companies. But running a complete voice stack itself is beyond the scope of Freescale. So vendors integrate their solutions with FSL solution. To test the framework we have a small application in our BSP (this is a very basic test client) which tests the TDM driver and the SLIC interface from voice transfer perspective. We can get this added in the Linux codebase in some test directory. What could be a good place for this? Regards Sandeep ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [2/3][PATCH][v2] TDM Framework
-Original Message- From: Mark Brown [mailto:broo...@opensource.wolfsonmicro.com] Sent: Monday, July 30, 2012 9:16 PM To: Francois Romieu Cc: Singh Sandeep-B37400; de...@driverdev.osuosl.org; linuxppc-dev@lists.ozlabs.org; ga...@kernel.crashing.org; linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org Subject: Re: [2/3][PATCH][v2] TDM Framework On Fri, Jul 27, 2012 at 05:25:42PM +0200, Francois Romieu wrote: 2. It would probably make sense to Cc: netdev and serial. There may be some kernel client network integration from the start. Plus audio, quite a few of the buses mentioned as examples of use cases for the hardware are audio ones. [Sandeep] Ok ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [2/3][PATCH][v2] TDM Framework
-Original Message- From: John Stoffel [mailto:j...@stoffel.org] Sent: 27 July 2012 19:42 To: Singh Sandeep-B37400 Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-ker...@lists.infradead.org; ga...@kernel.crashing.org; linux-ker...@vger.kernel.org; de...@driverdev.osuosl.org Subject: Re: [2/3][PATCH][v2] TDM Framework From: Sandeep Singh sand...@freescale.com TDM Framework is an attempt to provide a platform independent layer which can offer a standard interface for TDM access to different client modules. Please don't use TLAs (Three Letter Acronyms) like TDM without explaining the clearly and up front. It makes it hard for anyone else who doens't know your code to look it over without having to spend lots of time poking around to figure it out from either context or somewhere else. [Sandeep] Patch for documentation for TDM is present in this patch set, which explains TDM in detail. Should we do this in commit message too?? Link too documentation patch: http://patchwork.ozlabs.org/patch/173680/ John ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [2/3][PATCH][v2] TDM Framework
Thanks for your comments. Please find replies inline. Regards, Sandeep -Original Message- From: Francois Romieu [mailto:rom...@fr.zoreil.com] Sent: 27 July 2012 20:56 To: Singh Sandeep-B37400 Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-ker...@lists.infradead.org; ga...@kernel.crashing.org; linux-ker...@vger.kernel.org; de...@driverdev.osuosl.org Subject: Re: [2/3][PATCH][v2] TDM Framework sand...@freescale.com sand...@freescale.com : [...] The main functions of this Framework are: - provides interface to TDM clients to access TDM functionalities. - provides standard interface for TDM drivers to hook with the framework. - handles various data handling stuff and buffer management. In future this Framework will be extended to provide Interface for Line control devices also. For example SLIC, E1/T1 Framers etc. Presently the framework supports only Single Port channelised mode. Also the configurability options are limited which will be extended later on. Only kernel mode TDM clients are supported currently. Support for User mode clients will be added later. 1. You should send some kernel mode TDM clients. Without those the framework is pretty useless. [Sandeep] We do have a test client but not good enough to be pushed in open source, should we add it to documentation?? 2. It would probably make sense to Cc: netdev and serial. There may be some kernel client network integration from the start. [Sandeep] Ok. 3. Where is the userspace configuration interface ? [Sandeep] TDM framework right now supports only kernel mode clients. It has been tested with the client module that I mentioned above. Both the framework and test client are a part of Freescale BSP. [...] Based on: git://git.am.freescale.net/gitolite/mirrors/galak-powerpc.git [Sandeep] Please try below mentioned link. The above one is Freescale's internal mirror of: git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git $ git clone git://git.am.freescale.net/gitolite/mirrors/galak-powerpc.git Cloning into 'galak-powerpc'... fatal: Unable to look up git.am.freescale.net (port 9418) (No address associated with hostname) -- Ueimor ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [2/3][PATCH][v2] TDM Framework
Thanks for your comments. Please find my response inline. Regards, Sandeep -Original Message- From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk] Sent: Friday, July 27, 2012 8:22 PM To: Singh Sandeep-B37400 Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-ker...@lists.infradead.org; de...@driverdev.osuosl.org; ga...@kernel.crashing.org; linux-ker...@vger.kernel.org Subject: Re: [2/3][PATCH][v2] TDM Framework On Fri, Jul 27, 2012 at 07:35:38PM +0530, sand...@freescale.com wrote: +static DEFINE_MUTEX(tdm_core_lock); +static DEFINE_IDR(tdm_adapter_idr); +/* List of TDM adapters registered with TDM framework */ +LIST_HEAD(adapter_list); + +/* List of TDM clients registered with TDM framework */ +LIST_HEAD(driver_list); These two are far too generic to be public. Have you checked your code with sparse? I think not. [Sandeep] Will changes the name to be more appropriate. Right, I haven't checked with sparse. + +/* + * In case the previous data is not fetched by the client driver, the + * de-interleaving function will discard the old data and rewrite +the + * new data + */ + +static int use_latest_tdm_data = 1; + +/* Data structures required for sysfs */ static struct tdm_sysfs attr += { + .attr.name = use_latest_data, + .attr.mode = 0664, + .cmd_type = TDM_LATEST_DATA, +}; + +static struct attribute *tdm_attr[] = { + attr.attr, + NULL +}; + +const struct sysfs_ops tdm_ops = { + .show = tdm_show_sysfs, + .store = tdm_store_sysfs, +}; Again, lack of static. [Sandeep] Ok + +static struct kobj_type tdm_type = { + .sysfs_ops = tdm_ops, + .default_attrs = tdm_attr, +}; + +/* tries to match client driver with the adapter */ static int +tdm_device_match(struct tdm_driver *driver, struct tdm_adapter *adap) +{ + /* match on an id table if there is one */ + if (driver-id_table driver-id_table-name[0]) { + if (!(strcmp(driver-id_table-name, adap-name))) + return (int)driver-id_table; Casting a pointer to 'int' is not a good thing to do. Please fix this. [Sandeep] Will fix this. + } + return 0; +} + +static int tdm_attach_driver_adap(struct tdm_driver *driver, + struct tdm_adapter *adap) +{ + int ret = 0; + /* if driver is already attached to any other adapter, return*/ + if (driver-adapter (driver-adapter != adap)) Additional parens not required. [Sandeep] Ok + return 0; + + driver-adapter = adap; + + if (driver-attach_adapter) { + ret = driver-attach_adapter(adap); + if (ret 0) { + pr_err(tdm: attach_adapter failed for driver [%s] + err:%d\n, driver-name, ret); + return ret; + } + } + adap-drv_count++; + + if (!adap-tasklet_conf) { + tdm_sysfs_init(); + tasklet_init(adap-tdm_data_tasklet, tdm_data_tasklet_fn, + (unsigned long)adap); Why not init this tasklet when the struct tdm_adapter is first created? Why do you need to wait, and then have state tracking for this? [Sandeep] Ok, will take care + adap-tasklet_conf = 1; + } + + return ret; +} + +/* Detach client driver and adapter */ static int +tdm_detach_driver_adap(struct tdm_driver *driver, + struct tdm_adapter *adap) +{ + int res = 0; + + if (!driver-adapter || (driver-adapter != adap)) Additional parens not required. [Sandeep] Ok. + return 0; + + adap-drv_count--; + + /* If no more driver is registed with the adapter*/ + if (!adap-drv_count adap-tasklet_conf) { + tasklet_disable(adap-tdm_data_tasklet); + tasklet_kill(adap-tdm_data_tasklet); + adap-tasklet_conf = 0; + } + + if (driver-detach_adapter) { + if (driver-detach_adapter(adap)) + pr_err(tdm: detach_adapter failed for driver [%s]\n, + driver-name); + } + + driver-adapter = NULL; + return res; +} + +/* TDM adapter Registration/De-registration with TDM framework */ + +static int tdm_register_adapter(struct tdm_adapter *adap) { + int res = 0; + struct tdm_driver *driver, *next; + + mutex_init(adap-adap_lock); + INIT_LIST_HEAD(adap-myports); + spin_lock_init(adap-portlist_lock); + + adap-drv_count = 0; + adap-tasklet_conf = 0; + + list_add_tail(adap-list, adapter_list); What protects this list? [Sandeep] Ok, will take care + + /* initialization of driver by framework in default configuration */ + init_config_adapter(adap); + + /* Notify drivers */ + pr_info(adapter [%s] registered\n, adap-name); + mutex_lock(tdm_core_lock); + list_for_each_entry_safe(driver, next, driver_list, list
RE: [2/3][PATCH][upstream] TDM Framework
Hi, Please find my comment inline. Regards, Sandeep -Original Message- From: Tabi Timur-B04825 Sent: Tuesday, July 24, 2012 8:13 PM To: Singh Sandeep-B37400 Cc: linuxppc-dev@lists.ozlabs.org; Aggrwal Poonam-B10812 Subject: Re: [2/3][PATCH][upstream] TDM Framework Singh Sandeep-B37400 wrote: +int tdm_adap_send(struct tdm_adapter *adap, void **buf, int count) { + int res; + + if (adap-algo-tdm_write) + res = adap-algo-tdm_write(adap, buf, count); Why does tdm_write() return a u32? And shouldn't 'res' also be a u32, to make tdm_write()? [Sandeep] tdm_write() returns number of bytes written. You are right, 'res' should be declared as u32 Then it should return an unsigned int. You should used a sized integer type only when the size really matters (e.g. hardware registers or packed fields in a structure). [Sandeep] Agreed, will rectify. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [2/3][PATCH][upstream] TDM Framework
Hi Timur, Thanks for your comments, please find response inline. Regards, Sandeep -Original Message- From: Tabi Timur-B04825 Sent: Monday, July 23, 2012 10:03 PM To: Singh Sandeep-B37400 Cc: linuxppc-dev@lists.ozlabs.org; Singh Sandeep-B37400; Aggrwal Poonam-B10812 Subject: Re: [2/3][PATCH][upstream] TDM Framework On Mon, Jul 23, 2012 at 5:49 AM, b37...@freescale.com wrote: From: Sandeep Singh sand...@freescale.com Please fix your git configuration so that the From: line in your emails contains your full name. This patch was sent with this From: line: From: b37...@freescale.com It should say: From: Sandeep Singh sand...@freescale.com Three more things: 1) You don't need to add [upstream] to patches that are posted upstream. 2) This patch needs to be CC'd to linux-ker...@vger.kernel.org and de...@driverdev.osuosl.org. 3) I'm concerned that there is no struct device anywhere in this framework. I think that's a serious omission, and you need to figure out where you can put this. [Sandeep] 1) 2) Ok, Will take care. 3)Ok, will check. diff --git a/drivers/tdm/tdm-core.c b/drivers/tdm/tdm-core.c new file mode 100644 index 000..9973b6b --- /dev/null +++ b/drivers/tdm/tdm-core.c @@ -0,0 +1,1082 @@ +/* driver/tdm/tdm-core.c + * + * Copyright (C) 2012 Freescale Semiconductor, Inc, All rights reserved. I've been seeing this copyright messages a lot recently, and I don't understand why. This message is incorrectly formatted. The (C) is redundant, and the phrase All rights reserved. is wrong. This patch is licensed under the GPL, so we are NOT reserving all rights, we are actually giving up some rights. Please change this to: Copyright 2012 Freescale Semiconductor, Inc. [Sandeep] Will correct this. + * + * TDM core is the interface between TDM clients and TDM devices. + * It is also intended to serve as an interface for line controlled + * devices later on. + * + * Author:Hemant Agrawal hem...@freescale.com + * Rajesh Gumasta rajesh.guma...@freescale.com If these two are the authors, why are they not on the signed-off-by lines? [Sandeep] Will add them to signoff. + * + * Modified by Sandeep Kr Singh sand...@freescale.com + * Poonam Aggarwal poonam.aggar...@freescale.com Do not add modified by lines to the code. That's why we have a git history. [Sandeep] Ok. + * 1. Added framework based initialization of device. + * 2. All the init/run time configuration is now done by framework. + * 3. Added channel level operations. + * 4. Added sysfs knob to configure use_latest_tdm_data at runtime. Again, this is not information that belongs in the source file. [Sandeep] OK + * + * Note that some parts of this code may have been derived from i2c subsystem. + * + * This program is free software; you can redistribute it and/or + modify it + * under the terms of the GNU General Public License as published + by the + * Free Software Foundation; either version 2 of the License, or + (at your + * option) any later version. + * + * This program is distributed in the hope that it will be useful, + but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + along + * with this program; if not, write to the Free Software Foundation, + Inc., + * 675 Mass Ave, Cambridge, MA 02139, USA. + */ + + +#include linux/module.h +#include linux/kernel.h +#include linux/errno.h +#include linux/slab.h +#include linux/tdm.h +#include linux/init.h +#include linux/idr.h +#include linux/mutex.h +#include linux/completion.h +#include linux/hardirq.h +#include linux/irqflags.h +#include linux/list.h +#include linux/uaccess.h +#include linux/io.h Are you sure you need all of these header files? [Sandeep] Will check. + + +static DEFINE_MUTEX(tdm_core_lock); +static DEFINE_IDR(tdm_adapter_idr); +/* List of TDM adapters registered with TDM framework */ +LIST_HEAD(adapter_list); + +/* List of TDM clients registered with TDM framework */ +LIST_HEAD(driver_list); + +/* In case the previous data is not fetched by the client driver, the + * de-interleaving function will discard the old data and rewrite +the + * new data */ Proper multi-line comment format is like this: [Sandeep] Ok. /* * In case the previous data is not fetched by the client driver, the * de-interleaving function will discard the old data and rewrite the * new data. */ +static int use_latest_tdm_data = 1; + +/* Data structures required for sysfs */ static struct tdm_sysfs attr += { + .attr.name = use_latest_data, + .attr.mode = 0664, + .cmd_type = TDM_LATEST_DATA, +}; + +static struct attribute *tdm_attr[] = { + attr.attr, + NULL +}; + +const struct sysfs_ops tdm_ops = { + .show
RE: [PATCH][2/3][RFC] TDM Framework
-Original Message- From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org] Sent: Wednesday, April 25, 2012 11:20 AM To: Aggrwal Poonam-B10812 Cc: linuxppc-dev@lists.ozlabs.org; Singh Sandeep-B37400 Subject: Re: [PATCH][2/3][RFC] TDM Framework On Sat, 2012-03-10 at 18:27 +0530, Poonam Aggrwal wrote: From: Sandeep Singh sand...@freescale.com TDM Framework is an attempt to provide a platform independent layer which can offer a standard interface for TDM access to different client modules. Beneath, the framework layer can house different types of TDM drivers to handle various TDM devices, the hardware intricacies of the devices being completely taken care by TDM drivers. Neither the changeset comment, the code, not the Documentation file (which are non-existent, at least in this patch, though mentioned), define what TDM actually is :-) [Sandeep] Thanks for your comments. Documentation for TDM is present in the following patch: http://patchwork.ozlabs.org/patch/145857/ Regards, Sandeep Cheers, Ben. This framework layer will allow any type of TDM device to hook with it. For example Freescale controller as on MPC8315, UCC based TDM controller, or any other controller. The main functions of this Framework are: - provides interface to TDM clients to access TDM functionalities. - provides standard interface for TDM drivers to hook with the framework. - handles various data handling stuff and buffer management. In future this Framework will be extended to provide Interface for Line control devices also. For example SLIC, E1/T1 Framers etc. Limitations/Future Work --- 1. Presently the framework supports only Single Port channelised mode. 2. Also the configurability options are limited which will be extended later on. 3. Only kernel mode TDM clients are supported currently. Support for User mode clients will be added later. Signed-off-by: Sandeep Singh sand...@freescale.com Signed-off-by: Poonam Aggrwal poonam.aggr...@freescale.com --- A couple of todos' are left in the patch, we are working on it and will be addressed in the updated patch set. drivers/Kconfig |1 + drivers/Makefile|1 + drivers/tdm/Kconfig | 25 + drivers/tdm/tdm-core.c | 1146 +++ include/linux/mod_devicetable.h | 11 + include/linux/tdm.h | 347 6 files changed, 1531 insertions(+), 0 deletions(-) create mode 100644 drivers/tdm/Kconfig create mode 100644 drivers/tdm/tdm-core.c create mode 100644 include/linux/tdm.h diff --git a/drivers/Kconfig b/drivers/Kconfig index ad6c1eb..25f7f5b 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -130,4 +130,5 @@ source drivers/virt/Kconfig source drivers/net/dpa/NetCommSw/Kconfig +source drivers/tdm/Kconfig endmenu diff --git a/drivers/Makefile b/drivers/Makefile index cd546eb..362b5ed 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -102,6 +102,7 @@ obj-$(CONFIG_INFINIBAND) += infiniband/ obj-$(CONFIG_SGI_SN) += sn/ obj-y+= firmware/ obj-$(CONFIG_CRYPTO) += crypto/ +obj-$(CONFIG_TDM)+= tdm/ obj-$(CONFIG_SUPERH) += sh/ obj-$(CONFIG_ARCH_SHMOBILE) += sh/ ifndef CONFIG_ARCH_USES_GETTIMEOFFSET diff --git a/drivers/tdm/Kconfig b/drivers/tdm/Kconfig new file mode 100644 index 000..8db2b05 --- /dev/null +++ b/drivers/tdm/Kconfig @@ -0,0 +1,25 @@ +# +# TDM subsystem configuration +# + +menuconfig TDM + tristate TDM support + ---help--- + More information is contained in the directory file:Documentation/tdm/, + especially in the file called summary there. + If you want TDM support, you should say Y here and also to the + specific driver for your bus adapter(s) below. + + This TDM support can also be built as a module. If so, the module + will be called tdm-core. + +if TDM + +config TDM_DEBUG_CORE + bool TDM Core debugging messages + help + Say Y here if you want the TDM core to produce a bunch of debug + messages to the system log. Select this if you are having a + problem with TDM support and want to see more of what is going on. + +endif # TDM diff --git a/drivers/tdm/tdm-core.c b/drivers/tdm/tdm-core.c new file mode 100644 index 000..cdda260 --- /dev/null +++ b/drivers/tdm/tdm-core.c @@ -0,0 +1,1146 @@ +/* driver/tdm/tdm-core.c + * + * Copyright (C) 2012 Freescale Semiconductor, Inc, All rights reserved. + * + * TDM core is the interface between TDM clients and TDM devices. + * It is also intended to serve as an interface for line controld + * devices later on. + * + * Author:Hemant Agrawal hem...@freescale.com + * Rajesh Gumasta rajesh.guma...@freescale.com + * + * Modified by Sandeep Kr Singh sand...@freescale.com