[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1460564513.6620.170.camel@linux.intel.com>
Date: Wed, 13 Apr 2016 19:21:53 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Vinod Koul <vinod.koul@...el.com>
Cc: Viresh Kumar <vireshk@...nel.org>, linux-kernel@...r.kernel.org,
dmaengine@...r.kernel.org, Rob Herring <robh+dt@...nel.org>,
Hans-Christian Egtvedt <egtvedt@...fundet.no>,
Tejun Heo <tj@...nel.org>, Mark Brown <broonie@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Mark Rutland <mark.rutland@....com>,
Vineet Gupta <vgupta@...opsys.com>
Subject: Re: [PATCH v3 08/15] dmaengine: dw: revisit data_width property
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 <andriy.shevchenko@...ux.intel.com>
Intel Finland Oy
Powered by blists - more mailing lists