| Message ID | 1471347080-1411-1-git-send-email-Eugeniy.Paltsev@synopsys.com |
|---|---|
| State | New |
| Headers | show |
Hi Eugeniy,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.8-rc2 next-20160815]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Eugeniy-Paltsev/DW-Read-is_memcpy-and-is_nollp-property-from-device-tree/20160816-193459
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64
All errors (new ones prefixed by >>):
drivers/dma/dw/platform.c: In function 'dw_dma_parse_dt':
>> drivers/dma/dw/platform.c:136:8: error: 'struct dw_dma_platform_data' has no member named 'is_nollp'
pdata->is_nollp = true;
^
vim +136 drivers/dma/dw/platform.c
130 pdata->is_private = true;
131
132 if (of_property_read_bool(np, "is_memcpy"))
133 pdata->is_memcpy = true;
134
135 if (of_property_read_bool(np, "is_nollp"))
> 136 pdata->is_nollp = true;
137
138 if (!of_property_read_u32(np, "chan_allocation_order", &tmp))
139 pdata->chan_allocation_order = (unsigned char)tmp;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Tue, 2016-08-16 at 14:31 +0300, Eugeniy Paltsev wrote: > DW DMAC on ARC SDP became broken after df5c7386 ("dmaengine: dw: some > Intel > devices has no memcpy support") and 30cb2639 ("dmaengine: dw: don't > override > platform data with autocfg") commits. I'm not sure that word 'broken' is a correct one here. Is the platform code using this driver in the upstream already? If so, where is it located? > > * After df5c7386 commit "DMA_MEMCPY" capability option doesn't get set > correctly in platform driver version. > * After 30cb2639 commit "nollp" parameters don't get set correctly in > platform driver version. > > This happens because in old driver version there are three sources of > parameters: pdata, device tree and autoconfig hardware registers. Some > parameters were read from pdata and others from autoconfig hardware > registers. If pdata was absent some pdata structure fields were filled > with parameters from device tree. > But 30cb2639 commit disabled overriding pdata with autocfg, so if we > use platform driver version without pdata some parameters will not be > set. > This leads to inoperability of DW DMAC. My suggestion is still the same, i.e. split platform data to actual hardware properties and platform quirks. We might be able to use quirks even in case of auto configuration. > > This patch adds reading missed parameters from device tree. > > Note there's a prerequisite http://www.spinics.net/lists/dmaengine/msg > 10682.html > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > --- > drivers/dma/dw/platform.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c > index 5bda0eb..2712602 100644 > --- a/drivers/dma/dw/platform.c > +++ b/drivers/dma/dw/platform.c > @@ -129,6 +129,12 @@ dw_dma_parse_dt(struct platform_device *pdev) > if (of_property_read_bool(np, "is_private")) > pdata->is_private = true; > > + if (of_property_read_bool(np, "is_memcpy")) > + pdata->is_memcpy = true; > + > + if (of_property_read_bool(np, "is_nollp")) > + pdata->is_nollp = true; I'm pretty sure this one (besides that fact that it misses documentation update and '-' instead of '_' as ordered by DT policy) is unacceptable in DT since it represents *OS related* quirks. (Btw, is_private is also should not be there in the first place) Rob Herring (Cc'ed) might shed a light how to proceed in this case. > + > if (!of_property_read_u32(np, "chan_allocation_order", &tmp)) > pdata->chan_allocation_order = (unsigned char)tmp; >
On Fri, 2016-08-19 at 17:39 +0300, Andy Shevchenko wrote: > On Tue, 2016-08-16 at 14:31 +0300, Eugeniy Paltsev wrote: > > > > DW DMAC on ARC SDP became broken after df5c7386 ("dmaengine: dw: > > some > > Intel > > devices has no memcpy support") and 30cb2639 ("dmaengine: dw: don't > > override > > platform data with autocfg") commits. > I'm not sure that word 'broken' is a correct one here. Is the > platform > code using this driver in the upstream already? If so, where is it > located? > I'm not sure is it, but, at least, it changed driver behavior for ARC SDP boards. > > > > > > * After df5c7386 commit "DMA_MEMCPY" capability option doesn't get > > set > > correctly in platform driver version. > > * After 30cb2639 commit "nollp" parameters don't get set correctly > > in > > platform driver version. > > > > > > This happens because in old driver version there are three sources > > of > > parameters: pdata, device tree and autoconfig hardware registers. > > Some > > parameters were read from pdata and others from autoconfig hardware > > registers. If pdata was absent some pdata structure fields were > > filled > > with parameters from device tree. > > > > > But 30cb2639 commit disabled overriding pdata with autocfg, so if > > we > > use platform driver version without pdata some parameters will not > > be > > set. > > This leads to inoperability of DW DMAC. > My suggestion is still the same, i.e. split platform data to actual > hardware properties and platform quirks. We might be able to use > quirks > even in case of auto configuration. Do you have any idea about better way to do it? Do you suggest to split pdata structure in two different structures? > > > > > > > This patch adds reading missed parameters from device tree. > > > > Note there's a prerequisite http://www.spinics.net/lists/dmaengine/ > > msg > > 10682.html > > > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > > --- > > drivers/dma/dw/platform.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c > > index 5bda0eb..2712602 100644 > > --- a/drivers/dma/dw/platform.c > > +++ b/drivers/dma/dw/platform.c > > @@ -129,6 +129,12 @@ dw_dma_parse_dt(struct platform_device *pdev) > > if (of_property_read_bool(np, "is_private")) > > pdata->is_private = true; > > > > + if (of_property_read_bool(np, "is_memcpy")) > > + pdata->is_memcpy = true; > > + > > + if (of_property_read_bool(np, "is_nollp")) > > + pdata->is_nollp = true; > I'm pretty sure this one (besides that fact that it misses > documentation > update and '-' instead of '_' as ordered by DT policy) is > unacceptable > in DT since it represents *OS related* quirks. (Btw, is_private is > also > should not be there in the first place) Could you possibly tell me, why you call this quirks *OS related* ? For example: If I know, what DMAC in any chip on any board doesn't support memory- to-memory transfers, I can disable "is_memcpy" in this board .dts file. Am I wrong? > > Rob Herring (Cc'ed) might shed a light how to proceed in this case. > > > > > + > > if (!of_property_read_u32(np, "chan_allocation_order", > > &tmp)) > > pdata->chan_allocation_order = (unsigned char)tmp; > > -- Paltsev Eugeniy
On Tue, 2016-08-23 at 15:14 +0000, Eugeniy Paltsev wrote: > DW DMAC on ARC SDP became broken after df5c7386 ("dmaengine: dw: > > > some Intel devices has no memcpy support") and 30cb2639 > > > ("dmaengine: dw: don't override platform data with autocfg") > > > commits. > > I'm not sure that word 'broken' is a correct one here. Is the > > platform > > code using this driver in the upstream already? If so, where is it > > located? > > > I'm not sure is it, but, at least, it changed driver behavior for ARC > SDP boards. The rule of common sense here: if it was never upstreamed it has never been broken. I hardly remember any user of DW DMAC by ARC architecture in upstream. > > > But 30cb2639 commit disabled overriding pdata with autocfg, so if > > > we use platform driver version without pdata some parameters will > > > not be set. This leads to inoperability of DW DMAC. > > My suggestion is still the same, i.e. split platform data to actual > > hardware properties and platform quirks. We might be able to use > > quirks > > even in case of auto configuration. > Do you have any idea about better way to do it? > Do you suggest to split pdata structure in two different structures There might be at least couple of ways how to implement this. 1. Convert booleans to bits in one variable (let's say unsigned int quirks). 2. Split all quirks to separate quirks to something like struct dw_dma_platform_quirks. By obvious reasons I think first solution might be better. > > > + if (of_property_read_bool(np, "is_memcpy")) > > > + pdata->is_memcpy = true; > > > + > > > + if (of_property_read_bool(np, "is_nollp")) > > > + pdata->is_nollp = true; > > I'm pretty sure this one (besides that fact that it misses > > documentation update and '-' instead of '_' as ordered by DT > > policy) is unacceptable in DT since it represents *OS related* > > quirks. (Btw,is_private is also should not be there in the first > > place) > Could you possibly tell me, why you call this quirks *OS related* ? > For example: > If I know, what DMAC in any chip on any board doesn't support memory- > to-memory transfers, I can disable "is_memcpy" in this board .dts > file. > Am I wrong? Some of the properties are set or unset due to support in the driver and / or issues of the hardware _related_ to the driver in question. Though if anyone has different opinion I would appreciate to listen to.
On 08/23/2016 10:02 AM, Andy Shevchenko wrote: > On Tue, 2016-08-23 at 15:14 +0000, Eugeniy Paltsev wrote: > >> DW DMAC on ARC SDP became broken after df5c7386 ("dmaengine: dw: >>>> some Intel devices has no memcpy support") and 30cb2639 >>>> ("dmaengine: dw: don't override platform data with autocfg") >>>> commits. >>> I'm not sure that word 'broken' is a correct one here. Is the >>> platform >>> code using this driver in the upstream already? If so, where is it >>> located? >>> >> I'm not sure is it, but, at least, it changed driver behavior for ARC >> SDP boards. > The rule of common sense here: if it was never upstreamed it has never > been broken. Right ! > I hardly remember any user of DW DMAC by ARC architecture in upstream. The ARC SDP platform is provided by arch/arc/plat-axs and arch/arc/boot/ax* The IP Proto-typing kit folks here would just add a DT binding in there and things would just work out of the box - and that stopped recently - hence the notion of broken. But I agree one can't fix what can't be seen as broken. I just intervened to make this comment - I'm sure you and Eugeniy can agree on a workable solution. Thx, -Vineet
diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c index 5bda0eb..2712602 100644 --- a/drivers/dma/dw/platform.c +++ b/drivers/dma/dw/platform.c @@ -129,6 +129,12 @@ dw_dma_parse_dt(struct platform_device *pdev) if (of_property_read_bool(np, "is_private")) pdata->is_private = true; + if (of_property_read_bool(np, "is_memcpy")) + pdata->is_memcpy = true; + + if (of_property_read_bool(np, "is_nollp")) + pdata->is_nollp = true; + if (!of_property_read_u32(np, "chan_allocation_order", &tmp)) pdata->chan_allocation_order = (unsigned char)tmp;
DW DMAC on ARC SDP became broken after df5c7386 ("dmaengine: dw: some Intel devices has no memcpy support") and 30cb2639 ("dmaengine: dw: don't override platform data with autocfg") commits. * After df5c7386 commit "DMA_MEMCPY" capability option doesn't get set correctly in platform driver version. * After 30cb2639 commit "nollp" parameters don't get set correctly in platform driver version. This happens because in old driver version there are three sources of parameters: pdata, device tree and autoconfig hardware registers. Some parameters were read from pdata and others from autoconfig hardware registers. If pdata was absent some pdata structure fields were filled with parameters from device tree. But 30cb2639 commit disabled overriding pdata with autocfg, so if we use platform driver version without pdata some parameters will not be set. This leads to inoperability of DW DMAC. This patch adds reading missed parameters from device tree. Note there's a prerequisite http://www.spinics.net/lists/dmaengine/msg10682.html Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> --- drivers/dma/dw/platform.c | 6 ++++++ 1 file changed, 6 insertions(+)