From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754372AbcDMQVA (ORCPT ); Wed, 13 Apr 2016 12:21:00 -0400 Received: from mga02.intel.com ([134.134.136.20]:62066 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751059AbcDMQU7 (ORCPT ); Wed, 13 Apr 2016 12:20:59 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,480,1455004800"; d="scan'208";a="957887628" Message-ID: <1460564513.6620.170.camel@linux.intel.com> Subject: Re: [PATCH v3 08/15] dmaengine: dw: revisit data_width property From: Andy Shevchenko To: Vinod Koul Cc: Viresh Kumar , linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org, Rob Herring , Hans-Christian Egtvedt , Tejun Heo , Mark Brown , Greg Kroah-Hartman , Mark Rutland , Vineet Gupta Date: Wed, 13 Apr 2016 19:21:53 +0300 In-Reply-To: <20160413161715.GU2274@localhost> References: <1458311094-94927-1-git-send-email-andriy.shevchenko@linux.intel.com> <1458311094-94927-9-git-send-email-andriy.shevchenko@linux.intel.com> <20160413155719.GS2274@localhost> <1460563548.6620.165.camel@linux.intel.com> <20160413161715.GU2274@localhost> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.1-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2016-04-13 at 21:47 +0530, Vinod Koul wrote: > On Wed, Apr 13, 2016 at 07:05:48PM +0300, Andy Shevchenko wrote: > > > > On Wed, 2016-04-13 at 21:27 +0530, Vinod Koul wrote: > > > > > > On Fri, Mar 18, 2016 at 04:24:47PM +0200, Andy Shevchenko wrote: > > > The driver should still work with older DT implementation, so you > > > need > > > to > > > keep support for that in driver and hence I don't see any benfit > > > we > > > would > > > get from doing both in driver! > > The device tree is screwed by a process that allows to do almost > > whatever you want. Here I'm trying to rectify the usage of the > > field. > Well at least we should try to do the right thing which means pushing > back > on ABI breakage.. There is no such. > > > > > The old is still supported and benefit is apparently in unifying > > standard properties across the drivers. > Hrmmm how is that? The common usage for data-width property is "in bytes". And I like the idea. I don't know why at all I chose to keep encoded value there in the first place and no one commented at that time. I suppose because of screwed device tree process. I think now it's better to follow some standard / registered properties in new drivers. > > > > > > > > > > > > > > @@ -102,8 +102,8 @@ dw_dma_parse_dt(struct platform_device > > > > *pdev) > > > >  { > > > >   struct device_node *np = pdev->dev.of_node; > > > >   struct dw_dma_platform_data *pdata; > > > > - u32 tmp, arr[DW_DMA_MAX_NR_MASTERS]; > > > >   u32 nr_channels; > > > > + u32 tmp; > > > >   > > > >   if (!np) { > > > >   dev_err(&pdev->dev, "Missing DT data\n"); > > > > @@ -138,10 +138,10 @@ dw_dma_parse_dt(struct platform_device > > > > *pdev) > > > >   pdata->nr_masters = tmp; > > > >   } > > > >   > > > > - if (!of_property_read_u32_array(np, "data_width", arr, > > > > - pdata->nr_masters)) > > > > - for (tmp = 0; tmp < pdata->nr_masters; tmp++) > > > > - pdata->data_width[tmp] = arr[tmp]; > You stop reading the array Yes, due to "- Use one value for all AHB masters for now". No reason to read all of them. > > > > + if (!of_property_read_u32(np, "data-width", &tmp)) > > > > + pdata->data_width = tmp; > > > > + else if (!of_property_read_u32(np, "data_width", &tmp)) > > > > + pdata->data_width = BIT(tmp & 0x07); > And read a value? how will this work with older array? It will read first element. -- Andy Shevchenko Intel Finland Oy