linux-kernel - Re: [PATCH v3 08/15] dmaengine: dw: revisit data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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