From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Ujfalusi Subject: Re: [PATCH v3 01/10] platform_data: edma: Be precise with the paRAM struct Date: Tue, 27 May 2014 13:22:49 +0300 Message-ID: <53846779.4070204@ti.com> References: <1397475725-5036-1-git-send-email-peter.ujfalusi@ti.com> <1397475725-5036-2-git-send-email-peter.ujfalusi@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: Errors-To: davinci-linux-open-source-bounces-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org To: Olof Johansson Cc: "davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org" , "Koul, Vinod" , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Joel Fernandes , "dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Dan Williams , linux-omap , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: linux-omap@vger.kernel.org T24gMDUvMjcvMjAxNCAxMjozMiBBTSwgT2xvZiBKb2hhbnNzb24gd3JvdGU6Cj4gSGksCj4gCj4g T24gTW9uLCBBcHIgMTQsIDIwMTQgYXQgNDo0MSBBTSwgUGV0ZXIgVWpmYWx1c2kgPHBldGVyLnVq ZmFsdXNpQHRpLmNvbT4gd3JvdGU6Cj4+IFRoZSBlZG1hY2NfcGFyYW0gc3RydWN0IHNob3VsZCBm b2xsb3cgdGhlIGxheW91dCBvZiB0aGUgcGFSQU0gYXJlYSBpbiB0aGUKPj4gSFcuIEJlIGV4cGxp Y2l0IG9uIHRoZSBzaXplIG9mIHRoZSBmaWVsZHMgKHUzMikgYW5kIGFsc28gbWFyayB0aGUgc3Ry dWN0Cj4+IGFzIHBhY2tlZCB0byBhdm9pZCBhbnkgcGFkZGluZyBvbiBub24gMzJiaXQgYXJjaGl0 ZWN0dXJlcy4KPj4KPj4gU2lnbmVkLW9mZi1ieTogUGV0ZXIgVWpmYWx1c2kgPHBldGVyLnVqZmFs dXNpQHRpLmNvbT4KPj4gQWNrZWQtYnk6IEpvZWwgRmVybmFuZGVzIDxqb2VsZkB0aS5jb20+Cj4+ IC0tLQo+PiAgaW5jbHVkZS9saW51eC9wbGF0Zm9ybV9kYXRhL2VkbWEuaCB8IDE4ICsrKysrKysr Ky0tLS0tLS0tLQo+PiAgMSBmaWxlIGNoYW5nZWQsIDkgaW5zZXJ0aW9ucygrKSwgOSBkZWxldGlv bnMoLSkKPj4KPj4gZGlmZiAtLWdpdCBhL2luY2x1ZGUvbGludXgvcGxhdGZvcm1fZGF0YS9lZG1h LmggYi9pbmNsdWRlL2xpbnV4L3BsYXRmb3JtX2RhdGEvZWRtYS5oCj4+IGluZGV4IGY1MDgyMWNi NjRiZS4uOTIzZjhhM2U0Y2UwIDEwMDY0NAo+PiAtLS0gYS9pbmNsdWRlL2xpbnV4L3BsYXRmb3Jt X2RhdGEvZWRtYS5oCj4+ICsrKyBiL2luY2x1ZGUvbGludXgvcGxhdGZvcm1fZGF0YS9lZG1hLmgK Pj4gQEAgLTQzLDE1ICs0MywxNSBAQAo+Pgo+PiAgLyogUGFSQU0gc2xvdHMgYXJlIGxhaWQgb3V0 IGxpa2UgdGhpcyAqLwo+PiAgc3RydWN0IGVkbWFjY19wYXJhbSB7Cj4+IC0gICAgICAgdW5zaWdu ZWQgaW50IG9wdDsKPj4gLSAgICAgICB1bnNpZ25lZCBpbnQgc3JjOwo+PiAtICAgICAgIHVuc2ln bmVkIGludCBhX2JfY250Owo+PiAtICAgICAgIHVuc2lnbmVkIGludCBkc3Q7Cj4+IC0gICAgICAg dW5zaWduZWQgaW50IHNyY19kc3RfYmlkeDsKPj4gLSAgICAgICB1bnNpZ25lZCBpbnQgbGlua19i Y250cmxkOwo+PiAtICAgICAgIHVuc2lnbmVkIGludCBzcmNfZHN0X2NpZHg7Cj4+IC0gICAgICAg dW5zaWduZWQgaW50IGNjbnQ7Cj4+IC19Owo+PiArICAgICAgIHUzMiBvcHQ7Cj4+ICsgICAgICAg dTMyIHNyYzsKPj4gKyAgICAgICB1MzIgYV9iX2NudDsKPj4gKyAgICAgICB1MzIgZHN0Owo+PiAr ICAgICAgIHUzMiBzcmNfZHN0X2JpZHg7Cj4+ICsgICAgICAgdTMyIGxpbmtfYmNudHJsZDsKPj4g KyAgICAgICB1MzIgc3JjX2RzdF9jaWR4Owo+PiArICAgICAgIHUzMiBjY250Owo+PiArfSBfX3Bh Y2tlZDsKPj4KPj4gIC8qIGZpZWxkcyBpbiBlZG1hY2NfcGFyYW0ub3B0ICovCj4+ICAjZGVmaW5l IFNBTSAgICAgICAgICAgIEJJVCgwKQo+IAo+IEkgY2FtZSBhY3Jvc3MgdGhpcyBwYXRjaCB3aGVu IEkgd2FzIGxvb2tpbmcgYXQgYSBwdWxsIHJlcXVlc3QgZnJvbQo+IFNla2hhciBmb3IgRURNQSBj bGVhbnVwcywgYW5kIGl0IG1hZGUgbWUgbG9vayBjbG9zZXIgYXQgdGhlIGNvbnRlbnRzCj4gb2Yg dGhpcyBmaWxlLgo+IAo+IFRoZSBpbmNsdWRlL2xpbnV4L3BsYXRmb3JtX2RhdGEvIGRpcmVjdG9y eSBpcyBtZWFudCB0byBob2xkCj4gcGxhdGZvcm1fZGF0YSBkZWZpbml0aW9ucyBmb3IgZHJpdmVy cywgYW5kIG5vdGhpbmcgbW9yZS4KPiBwbGF0Zm9ybV9kYXRhL2VkbWEuaCBhbHNvIGNvbnRhaW5z IGEgd2hvbGUgYnVuY2ggb2YgaW50ZXJmYWNlCj4gZGVmaW5pdGlvbnMgZm9yIHRoZSBkcml2ZXIu IFRoZXkgZG8gbm90IGJlbG9uZyB0aGVyZSwgYW5kIHNob3VsZCBiZQo+IG1vdmVkIHRvIGEgZGlm ZmVyZW50IGluY2x1ZGUgZmlsZS4KPiAKPiBUaGF0IGFsc28gaW5jbHVkZXMgdGhlIGFib3ZlIHN0 cnVjdCwgYmVjYXVzZSBhcyBmYXIgYXMgSSBjYW4gdGVsbCBpdCdzCj4gYSBydW50aW1lIHN0YXRl IHN0cnVjdHVyZSwgbm90IHNvbWV0aGluZyB0aGF0IGlzIHBhc3NlZCBpbiB3aXRoCj4gcGxhdGZv cm0gZGF0YS4KPiAKPiBDYW4gc29tZW9uZSBwbGVhc2UgY2xlYW4gdGhpcyB1cD8gVGhhbmtzLgoK SSB0aGluayBKb2VsIGlzIHdvcmtpbmcgb24gdG8gbW92ZS9tZXJnZSB0aGUgY29kZSBmcm9tIGFy Y2gvYXJtL2NvbW1vbi9lZG1hLmMKdG8gZHJpdmVycy9kbWEvZWRtYS5jCkknbSBzdXJlIHdpdGhp biB0aGlzIHdvcmsgaGUgaXMgZ29pbmcgdG8gY2xlYW4gdXAgdGhlIGhlYWRlciBmaWxlIGFzIHdl bGwuCkFzIGEgZmlyc3Qgc3RlcCBJIHRoaW5rIHRoZSBub24gcGxhdGZvcm1fZGF0YSBjb250ZW50 IGNhbiBiZSBtb3ZlZCBhcwppbmNsdWRlL2xpbnV4L2VkbWEuaCBvciBwcm9iYWJseSBhcyB0aS1l ZG1hLmg/CgotLSAKUMOpdGVyCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fCkRhdmluY2ktbGludXgtb3Blbi1zb3VyY2UgbWFpbGluZyBsaXN0CkRhdmluY2kt bGludXgtb3Blbi1zb3VyY2VAbGludXguZGF2aW5jaWRzcC5jb20KaHR0cDovL2xpbnV4LmRhdmlu Y2lkc3AuY29tL21haWxtYW4vbGlzdGluZm8vZGF2aW5jaS1saW51eC1vcGVuLXNvdXJjZQo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: peter.ujfalusi@ti.com (Peter Ujfalusi) Date: Tue, 27 May 2014 13:22:49 +0300 Subject: [PATCH v3 01/10] platform_data: edma: Be precise with the paRAM struct In-Reply-To: References: <1397475725-5036-1-git-send-email-peter.ujfalusi@ti.com> <1397475725-5036-2-git-send-email-peter.ujfalusi@ti.com> Message-ID: <53846779.4070204@ti.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 05/27/2014 12:32 AM, Olof Johansson wrote: > Hi, > > On Mon, Apr 14, 2014 at 4:41 AM, Peter Ujfalusi wrote: >> The edmacc_param struct should follow the layout of the paRAM area in the >> HW. Be explicit on the size of the fields (u32) and also mark the struct >> as packed to avoid any padding on non 32bit architectures. >> >> Signed-off-by: Peter Ujfalusi >> Acked-by: Joel Fernandes >> --- >> include/linux/platform_data/edma.h | 18 +++++++++--------- >> 1 file changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h >> index f50821cb64be..923f8a3e4ce0 100644 >> --- a/include/linux/platform_data/edma.h >> +++ b/include/linux/platform_data/edma.h >> @@ -43,15 +43,15 @@ >> >> /* PaRAM slots are laid out like this */ >> struct edmacc_param { >> - unsigned int opt; >> - unsigned int src; >> - unsigned int a_b_cnt; >> - unsigned int dst; >> - unsigned int src_dst_bidx; >> - unsigned int link_bcntrld; >> - unsigned int src_dst_cidx; >> - unsigned int ccnt; >> -}; >> + u32 opt; >> + u32 src; >> + u32 a_b_cnt; >> + u32 dst; >> + u32 src_dst_bidx; >> + u32 link_bcntrld; >> + u32 src_dst_cidx; >> + u32 ccnt; >> +} __packed; >> >> /* fields in edmacc_param.opt */ >> #define SAM BIT(0) > > I came across this patch when I was looking at a pull request from > Sekhar for EDMA cleanups, and it made me look closer at the contents > of this file. > > The include/linux/platform_data/ directory is meant to hold > platform_data definitions for drivers, and nothing more. > platform_data/edma.h also contains a whole bunch of interface > definitions for the driver. They do not belong there, and should be > moved to a different include file. > > That also includes the above struct, because as far as I can tell it's > a runtime state structure, not something that is passed in with > platform data. > > Can someone please clean this up? Thanks. I think Joel is working on to move/merge the code from arch/arm/common/edma.c to drivers/dma/edma.c I'm sure within this work he is going to clean up the header file as well. As a first step I think the non platform_data content can be moved as include/linux/edma.h or probably as ti-edma.h? -- P?ter From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752339AbaE0KXS (ORCPT ); Tue, 27 May 2014 06:23:18 -0400 Received: from devils.ext.ti.com ([198.47.26.153]:50980 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751705AbaE0KXR (ORCPT ); Tue, 27 May 2014 06:23:17 -0400 Message-ID: <53846779.4070204@ti.com> Date: Tue, 27 May 2014 13:22:49 +0300 From: Peter Ujfalusi User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: Olof Johansson CC: Dan Williams , "Koul, Vinod" , Sekhar Nori , Joel Fernandes , "dmaengine@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , linux-omap , "davinci-linux-open-source@linux.davincidsp.com" Subject: Re: [PATCH v3 01/10] platform_data: edma: Be precise with the paRAM struct References: <1397475725-5036-1-git-send-email-peter.ujfalusi@ti.com> <1397475725-5036-2-git-send-email-peter.ujfalusi@ti.com> In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/27/2014 12:32 AM, Olof Johansson wrote: > Hi, > > On Mon, Apr 14, 2014 at 4:41 AM, Peter Ujfalusi wrote: >> The edmacc_param struct should follow the layout of the paRAM area in the >> HW. Be explicit on the size of the fields (u32) and also mark the struct >> as packed to avoid any padding on non 32bit architectures. >> >> Signed-off-by: Peter Ujfalusi >> Acked-by: Joel Fernandes >> --- >> include/linux/platform_data/edma.h | 18 +++++++++--------- >> 1 file changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h >> index f50821cb64be..923f8a3e4ce0 100644 >> --- a/include/linux/platform_data/edma.h >> +++ b/include/linux/platform_data/edma.h >> @@ -43,15 +43,15 @@ >> >> /* PaRAM slots are laid out like this */ >> struct edmacc_param { >> - unsigned int opt; >> - unsigned int src; >> - unsigned int a_b_cnt; >> - unsigned int dst; >> - unsigned int src_dst_bidx; >> - unsigned int link_bcntrld; >> - unsigned int src_dst_cidx; >> - unsigned int ccnt; >> -}; >> + u32 opt; >> + u32 src; >> + u32 a_b_cnt; >> + u32 dst; >> + u32 src_dst_bidx; >> + u32 link_bcntrld; >> + u32 src_dst_cidx; >> + u32 ccnt; >> +} __packed; >> >> /* fields in edmacc_param.opt */ >> #define SAM BIT(0) > > I came across this patch when I was looking at a pull request from > Sekhar for EDMA cleanups, and it made me look closer at the contents > of this file. > > The include/linux/platform_data/ directory is meant to hold > platform_data definitions for drivers, and nothing more. > platform_data/edma.h also contains a whole bunch of interface > definitions for the driver. They do not belong there, and should be > moved to a different include file. > > That also includes the above struct, because as far as I can tell it's > a runtime state structure, not something that is passed in with > platform data. > > Can someone please clean this up? Thanks. I think Joel is working on to move/merge the code from arch/arm/common/edma.c to drivers/dma/edma.c I'm sure within this work he is going to clean up the header file as well. As a first step I think the non platform_data content can be moved as include/linux/edma.h or probably as ti-edma.h? -- Péter