diff for duplicates of <1477583708.5295.35.camel@linux.intel.com> diff --git a/a/1.txt b/N1/1.txt index 9b79e86..c2a72f6 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,4 +1,4 @@ -On Thu, 2016-10-27@18:34 +0300, Eugeniy Paltsev wrote: +On Thu, 2016-10-27 at 18:34 +0300, Eugeniy Paltsev wrote: > This patch is to address a proposal by Andy in this thread: > http://www.spinics.net/lists/dmaengine/msg11506.html > Split platform data to actual hardware properties, and platform @@ -11,11 +11,11 @@ Thanks for an update, my comments below. > --- > Changes for v2: -> ???- use separate bool values for quirks in "dw_dma_platform_data" +> - use separate bool values for quirks in "dw_dma_platform_data" > instead of -> ?????common bit field. +> common bit field. -> ???- convert device tree properties reading to unified device property +> - convert device tree properties reading to unified device property > API. This should be a separate patch. @@ -23,7 +23,7 @@ This should be a separate patch. > > I was wrong about DW_DMA_IS_SOFT_LLP flag - it is used to check about > ongoing soft llp transfer. So DW_DMA_IS_SOFT_LLP flag and "dwc- -> >nollp"? +> >nollp" > variable have different functions and I couldn't just get rid of "dwc- > >nollp" > and use DW_DMA_IS_SOFT_LLP flag instead. So I left "dwc->nollp" @@ -37,14 +37,14 @@ But this is other story independent of the subject. > --- a/drivers/dma/dw/core.c > +++ b/drivers/dma/dw/core.c > @@ -1452,9 +1452,24 @@ int dw_dma_probe(struct dw_dma_chip *chip) -> ? dw->regs = chip->regs; -> ? chip->dw = dw; -> ? +> dw->regs = chip->regs; +> chip->dw = dw; +> > + /* Reassign the platform data pointer */ > + pdata = dw->pdata; > + -> ? pm_runtime_get_sync(chip->dev); -> ? +> pm_runtime_get_sync(chip->dev); +> > - if (!chip->pdata) { > + if ((!chip->pdata) || (chip->pdata && chip->pdata- > >only_quirks_used)) { @@ -54,10 +54,10 @@ if (!chip->pdata || chip->pdata->only_quirks_used) > + if (!chip->pdata) { > + /* -> + ?* Fill quirks with the default values in +> + * Fill quirks with the default values in > case of -> + ?* pdata absence. -> + ?*/ +> + * pdata absence. +> + */ > + pdata->is_private = true; > + pdata->is_memcpy = true; > + } else { @@ -69,30 +69,30 @@ Would we leave the first part in the place it is now and add new piece after? > + -> ? dw_params = dma_readl(dw, DW_PARAMS); -> ? dev_dbg(chip->dev, "DW_PARAMS: 0x%08x\n", dw_params); -> ? +> dw_params = dma_readl(dw, DW_PARAMS); +> dev_dbg(chip->dev, "DW_PARAMS: 0x%08x\n", dw_params); +> > @@ -1464,9 +1479,6 @@ int dw_dma_probe(struct dw_dma_chip *chip) -> ? goto err_pdata; -> ? } -> ? +> goto err_pdata; +> } +> > - /* Reassign the platform data pointer */ > - pdata = dw->pdata; > - -> ? /* Get hardware configuration parameters */ -> ? pdata->nr_channels = (dw_params >> DW_PARAMS_NR_CHAN +> /* Get hardware configuration parameters */ +> pdata->nr_channels = (dw_params >> DW_PARAMS_NR_CHAN > & 7) + 1; -> ? pdata->nr_masters = (dw_params >> DW_PARAMS_NR_MASTER +> pdata->nr_masters = (dw_params >> DW_PARAMS_NR_MASTER > & 3) + 1; > @@ -1477,8 +1489,6 @@ int dw_dma_probe(struct dw_dma_chip *chip) -> ? pdata->block_size = dma_readl(dw, MAX_BLK_SIZE); -> ? -> ? /* Fill platform data with the default values */ +> pdata->block_size = dma_readl(dw, MAX_BLK_SIZE); +> +> /* Fill platform data with the default values */ > - pdata->is_private = true; > - pdata->is_memcpy = true; -> ? pdata->chan_allocation_order = +> pdata->chan_allocation_order = > CHAN_ALLOCATION_ASCENDING; -> ? pdata->chan_priority = CHAN_PRIORITY_ASCENDING; +> pdata->chan_priority = CHAN_PRIORITY_ASCENDING; ...like @@ -109,9 +109,9 @@ if (chip->data && chip->data->only_quirks_used) { As I mentioned above, please do a separate patch for this. > @@ -183,7 +186,7 @@ static int dw_probe(struct platform_device *pdev) -> ? -> ? pdata = dev_get_platdata(dev); -> ? if (!pdata) +> +> pdata = dev_get_platdata(dev); +> if (!pdata) > - pdata = dw_dma_parse_dt(pdev); > + pdata = dw_dma_parse_dt(dev); @@ -128,30 +128,30 @@ dw_dma_parse_properties(dev); Can you somehow be more clear that all listed quirks will be copied from platform data. -> ? * @is_nollp: The device channels does not support multi block +> * @is_nollp: The device channels does not support multi block > transfers. -> ? * @chan_allocation_order: Allocate channels starting from 0 or 7 -> ? * @chan_priority: Set channel priority increasing from 0 to 7 or 7 +> * @chan_allocation_order: Allocate channels starting from 0 or 7 +> * @chan_priority: Set channel priority increasing from 0 to 7 or 7 > to 0. > @@ -52,6 +55,7 @@ struct dw_dma_platform_data { -> ? unsigned int nr_channels; -> ? bool is_private; -> ? bool is_memcpy; +> unsigned int nr_channels; +> bool is_private; +> bool is_memcpy; > + bool only_quirks_used; -Perhaps add if at the end of quirk list and name just? +Perhaps add if at the end of quirk list and name just -> ? bool is_nollp; +> bool is_nollp; ...here bool use_quirks; -> ?#define CHAN_ALLOCATION_ASCENDING 0 /* zero to seven */ -> ?#define CHAN_ALLOCATION_DESCENDING 1 /* seven to zero +> #define CHAN_ALLOCATION_ASCENDING 0 /* zero to seven */ +> #define CHAN_ALLOCATION_DESCENDING 1 /* seven to zero > */ -- -Andy Shevchenko <andriy.shevchenko at linux.intel.com> +Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy diff --git a/a/content_digest b/N1/content_digest index bb5b739..7c3845c 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,11 +1,17 @@ "ref\01477582497-19302-1-git-send-email-Eugeniy.Paltsev@synopsys.com\0" - "From\0andriy.shevchenko@linux.intel.com (Andy Shevchenko)\0" - "Subject\0[PATCH v2] dmaengine: DW DMAC: split pdata to hardware properties and platform quirks\0" + "From\0Andy Shevchenko <andriy.shevchenko@linux.intel.com>\0" + "Subject\0Re: [PATCH v2] dmaengine: DW DMAC: split pdata to hardware properties and platform quirks\0" "Date\0Thu, 27 Oct 2016 18:55:08 +0300\0" - "To\0linux-snps-arc@lists.infradead.org\0" + "To\0Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>" + " dmaengine@vger.kernel.org\0" + "Cc\0linux-kernel@vger.kernel.org" + vinod.koul@intel.com + dan.j.williams@intel.com + vireshk@kernel.org + " linux-snps-arc@lists.infradead.org\0" "\00:1\0" "b\0" - "On Thu, 2016-10-27@18:34 +0300, Eugeniy Paltsev wrote:\n" + "On Thu, 2016-10-27 at 18:34 +0300, Eugeniy Paltsev wrote:\n" "> This patch is to address a proposal by Andy in this thread:\n" "> http://www.spinics.net/lists/dmaengine/msg11506.html\n" "> Split platform data to actual hardware properties, and platform\n" @@ -18,11 +24,11 @@ "\n" "> ---\n" "> Changes for v2:\n" - "> ???- use separate bool values for quirks in \"dw_dma_platform_data\"\n" + "> \302\240\302\240\302\240- use separate bool values for quirks in \"dw_dma_platform_data\"\n" "> instead of\n" - "> ?????common bit field.\n" + "> \302\240\302\240\302\240\302\240\302\240common bit field.\n" "\n" - "> ???- convert device tree properties reading to unified device property\n" + "> \302\240\302\240\302\240- convert device tree properties reading to unified device property\n" "> API.\n" "\n" "This should be a separate patch.\n" @@ -30,7 +36,7 @@ "> \n" "> I was wrong about DW_DMA_IS_SOFT_LLP flag - it is used to check about\n" "> ongoing soft llp transfer. So DW_DMA_IS_SOFT_LLP flag and \"dwc-\n" - "> >nollp\"?\n" + "> >nollp\"\302\240\n" "> variable have different functions and I couldn't just get rid of \"dwc-\n" "> >nollp\"\n" "> and use DW_DMA_IS_SOFT_LLP flag instead. So I left \"dwc->nollp\"\n" @@ -44,14 +50,14 @@ "> --- a/drivers/dma/dw/core.c\n" "> +++ b/drivers/dma/dw/core.c\n" "> @@ -1452,9 +1452,24 @@ int dw_dma_probe(struct dw_dma_chip *chip)\n" - "> ?\tdw->regs = chip->regs;\n" - "> ?\tchip->dw = dw;\n" - "> ?\n" + "> \302\240\tdw->regs = chip->regs;\n" + "> \302\240\tchip->dw = dw;\n" + "> \302\240\n" "> +\t/* Reassign the platform data pointer */\n" "> +\tpdata = dw->pdata;\n" "> +\n" - "> ?\tpm_runtime_get_sync(chip->dev);\n" - "> ?\n" + "> \302\240\tpm_runtime_get_sync(chip->dev);\n" + "> \302\240\n" "> -\tif (!chip->pdata) {\n" "> +\tif ((!chip->pdata) || (chip->pdata && chip->pdata-\n" "> >only_quirks_used)) {\n" @@ -61,10 +67,10 @@ "\n" "> +\t\tif (!chip->pdata) {\n" "> +\t\t\t/*\n" - "> +\t\t\t?* Fill quirks with the default values in\n" + "> +\t\t\t\302\240* Fill quirks with the default values in\n" "> case of\n" - "> +\t\t\t?* pdata absence.\n" - "> +\t\t\t?*/\n" + "> +\t\t\t\302\240* pdata absence.\n" + "> +\t\t\t\302\240*/\n" "> +\t\t\tpdata->is_private = true;\n" "> +\t\t\tpdata->is_memcpy = true;\n" "> +\t\t} else {\n" @@ -76,30 +82,30 @@ "after?\n" "\n" "> +\n" - "> ?\t\tdw_params = dma_readl(dw, DW_PARAMS);\n" - "> ?\t\tdev_dbg(chip->dev, \"DW_PARAMS: 0x%08x\\n\", dw_params);\n" - "> ?\n" + "> \302\240\t\tdw_params = dma_readl(dw, DW_PARAMS);\n" + "> \302\240\t\tdev_dbg(chip->dev, \"DW_PARAMS: 0x%08x\\n\", dw_params);\n" + "> \302\240\n" "> @@ -1464,9 +1479,6 @@ int dw_dma_probe(struct dw_dma_chip *chip)\n" - "> ?\t\t\tgoto err_pdata;\n" - "> ?\t\t}\n" - "> ?\n" + "> \302\240\t\t\tgoto err_pdata;\n" + "> \302\240\t\t}\n" + "> \302\240\n" "> -\t\t/* Reassign the platform data pointer */\n" "> -\t\tpdata = dw->pdata;\n" "> -\n" - "> ?\t\t/* Get hardware configuration parameters */\n" - "> ?\t\tpdata->nr_channels = (dw_params >> DW_PARAMS_NR_CHAN\n" + "> \302\240\t\t/* Get hardware configuration parameters */\n" + "> \302\240\t\tpdata->nr_channels = (dw_params >> DW_PARAMS_NR_CHAN\n" "> & 7) + 1;\n" - "> ?\t\tpdata->nr_masters = (dw_params >> DW_PARAMS_NR_MASTER\n" + "> \302\240\t\tpdata->nr_masters = (dw_params >> DW_PARAMS_NR_MASTER\n" "> & 3) + 1;\n" "> @@ -1477,8 +1489,6 @@ int dw_dma_probe(struct dw_dma_chip *chip)\n" - "> ?\t\tpdata->block_size = dma_readl(dw, MAX_BLK_SIZE);\n" - "> ?\n" - "> ?\t\t/* Fill platform data with the default values */\n" + "> \302\240\t\tpdata->block_size = dma_readl(dw, MAX_BLK_SIZE);\n" + "> \302\240\n" + "> \302\240\t\t/* Fill platform data with the default values */\n" "> -\t\tpdata->is_private = true;\n" "> -\t\tpdata->is_memcpy = true;\n" - "> ?\t\tpdata->chan_allocation_order =\n" + "> \302\240\t\tpdata->chan_allocation_order =\n" "> CHAN_ALLOCATION_ASCENDING;\n" - "> ?\t\tpdata->chan_priority = CHAN_PRIORITY_ASCENDING;\n" + "> \302\240\t\tpdata->chan_priority = CHAN_PRIORITY_ASCENDING;\n" "\n" "...like\n" "\n" @@ -116,9 +122,9 @@ "As I mentioned above, please do a separate patch for this.\n" "\n" "> @@ -183,7 +186,7 @@ static int dw_probe(struct platform_device *pdev)\n" - "> ?\n" - "> ?\tpdata = dev_get_platdata(dev);\n" - "> ?\tif (!pdata)\n" + "> \302\240\n" + "> \302\240\tpdata = dev_get_platdata(dev);\n" + "> \302\240\tif (!pdata)\n" "> -\t\tpdata = dw_dma_parse_dt(pdev);\n" "> +\t\tpdata = dw_dma_parse_dt(dev);\n" "\n" @@ -135,32 +141,32 @@ "Can you somehow be more clear that all listed quirks will be copied from\n" "platform data.\n" "\n" - "> ? * @is_nollp: The device channels does not support multi block\n" + "> \302\240 * @is_nollp: The device channels does not support multi block\n" "> transfers.\n" - "> ? * @chan_allocation_order: Allocate channels starting from 0 or 7\n" - "> ? * @chan_priority: Set channel priority increasing from 0 to 7 or 7\n" + "> \302\240 * @chan_allocation_order: Allocate channels starting from 0 or 7\n" + "> \302\240 * @chan_priority: Set channel priority increasing from 0 to 7 or 7\n" "> to 0.\n" "> @@ -52,6 +55,7 @@ struct dw_dma_platform_data {\n" - "> ?\tunsigned int\tnr_channels;\n" - "> ?\tbool\t\tis_private;\n" - "> ?\tbool\t\tis_memcpy;\n" + "> \302\240\tunsigned int\tnr_channels;\n" + "> \302\240\tbool\t\tis_private;\n" + "> \302\240\tbool\t\tis_memcpy;\n" "\n" "> +\tbool\t\tonly_quirks_used;\n" "\n" - "Perhaps add if at the end of quirk list and name just?\n" + "Perhaps add if at the end of quirk list and name just\302\240\n" "\n" - "> ?\tbool\t\tis_nollp;\n" + "> \302\240\tbool\t\tis_nollp;\n" "\n" "...here\n" "\n" "bool use_quirks;\n" "\n" - "> ?#define CHAN_ALLOCATION_ASCENDING\t0\t/* zero to seven */\n" - "> ?#define CHAN_ALLOCATION_DESCENDING\t1\t/* seven to zero\n" + "> \302\240#define CHAN_ALLOCATION_ASCENDING\t0\t/* zero to seven */\n" + "> \302\240#define CHAN_ALLOCATION_DESCENDING\t1\t/* seven to zero\n" "> */\n" "\n" "-- \n" - "Andy Shevchenko <andriy.shevchenko at linux.intel.com>\n" + "Andy Shevchenko <andriy.shevchenko@linux.intel.com>\n" Intel Finland Oy -8a00d0a18efca082fd4f23b69269faf5ed058c213ddedcbe2dcd973fabd584dc +ebee8c63ad03e7aa176c3ad764ce9093a6c0b6d97548b8ee750e03cdc8e38711
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.