Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754678AbcJEPPQ (ORCPT ); Wed, 5 Oct 2016 11:15:16 -0400 Received: from us01smtprelay-2.synopsys.com ([198.182.47.9]:50877 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751647AbcJEPPO (ORCPT ); Wed, 5 Oct 2016 11:15:14 -0400 From: Eugeniy Paltsev To: "andriy.shevchenko@linux.intel.com" CC: "dan.j.williams@intel.com" , "linux-kernel@vger.kernel.org" , "Eugeniy.Paltsev@synopsys.com" , "vireshk@kernel.org" , "vinod.koul@intel.com" , "linux-snps-arc@lists.infradead.org" , "dmaengine@vger.kernel.org" Subject: Re: [PATCH] dmaengine: DW DMAC: split pdata to hardware properties and platform quirks Thread-Topic: [PATCH] dmaengine: DW DMAC: split pdata to hardware properties and platform quirks Thread-Index: AQHSD1MTEg7g+vAoAEKwDtC0EjT2sqCZ9iIA Date: Wed, 5 Oct 2016 15:14:25 +0000 Message-ID: <1475680464.7320.44.camel@synopsys.com> References: <1473945253-16649-1-git-send-email-Eugeniy.Paltsev@synopsys.com> In-Reply-To: <1473945253-16649-1-git-send-email-Eugeniy.Paltsev@synopsys.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.121.14.112] Content-Type: text/plain; charset="utf-8" Content-ID: <051D63F80F8402408349393938A74287@internal.synopsys.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id u95FFLJv013215 Content-Length: 8033 Lines: 241 Hi Andy, what do you think about these changes? On Thu, 2016-09-15 at 16:14 +0300, Eugeniy Paltsev wrote: > This patch is to address a proposal by Andy in this thread: > http://www.spinics.net/lists/dmaengine/msg10754.html > Split platform data to actual hardware properties, and platform > quirks. > Now we able to use quirks and hardware properties separately from > different sources (pdata, device tree or autoconfig registers) > > Signed-off-by: Eugeniy Paltsev > --- >  drivers/dma/dw/core.c                | 31 +++++++++++++++----------- >  drivers/dma/dw/platform.c            | 42 +++++++++++++++++++++----- > ---------- >  include/linux/platform_data/dma-dw.h | 20 +++++++++++------ >  3 files changed, 57 insertions(+), 36 deletions(-) > > diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c > index c2c0a61..9352735 100644 > --- a/drivers/dma/dw/core.c > +++ b/drivers/dma/dw/core.c > @@ -1451,10 +1451,25 @@ int dw_dma_probe(struct dw_dma_chip *chip) >   >   dw->regs = chip->regs; >   chip->dw = dw; > + /* Reassign the platform data pointer */ > + pdata = dw->pdata; >   >   pm_runtime_get_sync(chip->dev); >   > - if (!chip->pdata) { > + if ((!chip->pdata) || > +    (chip->pdata && test_bit(QUIRKS_ONLY_USED, &chip->pdata- > >quirks))) { > + > + /* > +  * Fill quirks with the default values in case of > pdata absence > +  */ > + if (!chip->pdata) { > + set_bit(QUIRKS_IS_PRIVATE, &pdata->quirks); > + set_bit(QUIRKS_IS_MEMCPY, &pdata->quirks); > + set_bit(QUIRKS_IS_NOLLP, &pdata->quirks); > + } else { > + pdata->quirks = chip->pdata->quirks; > + } > + >   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; >   } >   > - /* Reassign the platform data pointer */ > - pdata = dw->pdata; > - >   /* Get hardware configuration parameters */ >   pdata->nr_channels = (dw_params >> DW_PARAMS_NR_CHAN > & 7) + 1; >   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->is_private = true; > - pdata->is_memcpy = true; >   pdata->chan_allocation_order = > CHAN_ALLOCATION_ASCENDING; >   pdata->chan_priority = CHAN_PRIORITY_ASCENDING; >   } else if (chip->pdata->nr_channels > > DW_DMA_MAX_NR_CHANNELS) { > @@ -1486,9 +1496,6 @@ int dw_dma_probe(struct dw_dma_chip *chip) >   goto err_pdata; >   } else { >   memcpy(dw->pdata, chip->pdata, sizeof(*dw->pdata)); > - > - /* Reassign the platform data pointer */ > - pdata = dw->pdata; >   } >   >   dw->chan = devm_kcalloc(chip->dev, pdata->nr_channels, > sizeof(*dw->chan), > @@ -1569,7 +1576,7 @@ int dw_dma_probe(struct dw_dma_chip *chip) >   (dwc_params >> DWC_PARAMS_MBLK_EN & > 0x1) == 0; >   } else { >   dwc->block_size = pdata->block_size; > - dwc->nollp = pdata->is_nollp; > + dwc->nollp = test_bit(QUIRKS_IS_NOLLP, > &pdata->quirks); >   } >   } >   > @@ -1582,9 +1589,9 @@ int dw_dma_probe(struct dw_dma_chip *chip) >   >   /* Set capabilities */ >   dma_cap_set(DMA_SLAVE, dw->dma.cap_mask); > - if (pdata->is_private) > + if (test_bit(QUIRKS_IS_PRIVATE, &pdata->quirks)) >   dma_cap_set(DMA_PRIVATE, dw->dma.cap_mask); > - if (pdata->is_memcpy) > + if (test_bit(QUIRKS_IS_MEMCPY, &pdata->quirks)) >   dma_cap_set(DMA_MEMCPY, dw->dma.cap_mask); >   >   dw->dma.dev = chip->dev; > diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c > index 5bda0eb..308b977 100644 > --- a/drivers/dma/dw/platform.c > +++ b/drivers/dma/dw/platform.c > @@ -12,6 +12,7 @@ >   * published by the Free Software Foundation. >   */ >   > +#include >  #include >  #include >  #include > @@ -111,41 +112,48 @@ dw_dma_parse_dt(struct platform_device *pdev) >   return NULL; >   } >   > - if (of_property_read_u32(np, "dma-masters", &nr_masters)) > - return NULL; > - if (nr_masters < 1 || nr_masters > DW_DMA_MAX_NR_MASTERS) > - return NULL; > - > - if (of_property_read_u32(np, "dma-channels", &nr_channels)) > - return NULL; > - >   pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), > GFP_KERNEL); >   if (!pdata) >   return NULL; >   > + set_bit(QUIRKS_ONLY_USED, &pdata->quirks); > + > + if (of_property_read_bool(np, "is-private")) > + set_bit(QUIRKS_IS_PRIVATE, &pdata->quirks); > + > + if (of_property_read_bool(np, "is-memcpy")) > + set_bit(QUIRKS_IS_MEMCPY, &pdata->quirks); > + > + if (of_property_read_bool(np, "is-nollp")) > + set_bit(QUIRKS_IS_NOLLP, &pdata->quirks); > + > + if (of_property_read_u32(np, "dma-masters", &nr_masters)) > + return pdata; > + if (nr_masters < 1 || nr_masters > DW_DMA_MAX_NR_MASTERS) > + return pdata; > + >   pdata->nr_masters = nr_masters; > - pdata->nr_channels = nr_channels; >   > - if (of_property_read_bool(np, "is_private")) > - pdata->is_private = true; > + if (of_property_read_u32(np, "dma-channels", &nr_channels)) > + return pdata; >   > - if (!of_property_read_u32(np, "chan_allocation_order", > &tmp)) > + pdata->nr_channels = nr_channels; > + > + if (!of_property_read_u32(np, "chan-allocation-order", > &tmp)) >   pdata->chan_allocation_order = (unsigned char)tmp; >   > - if (!of_property_read_u32(np, "chan_priority", &tmp)) > + if (!of_property_read_u32(np, "chan-priority", &tmp)) >   pdata->chan_priority = tmp; >   > - if (!of_property_read_u32(np, "block_size", &tmp)) > + if (!of_property_read_u32(np, "block-size", &tmp)) >   pdata->block_size = tmp; >   >   if (!of_property_read_u32_array(np, "data-width", arr, > nr_masters)) { >   for (tmp = 0; tmp < nr_masters; tmp++) >   pdata->data_width[tmp] = arr[tmp]; > - } else if (!of_property_read_u32_array(np, "data_width", > arr, nr_masters)) { > - for (tmp = 0; tmp < nr_masters; tmp++) > - pdata->data_width[tmp] = BIT(arr[tmp] & > 0x07); >   } >   > + clear_bit(QUIRKS_ONLY_USED, &pdata->quirks); >   return pdata; >  } >  #else > diff --git a/include/linux/platform_data/dma-dw.h > b/include/linux/platform_data/dma-dw.h > index 5f0e11e..9cd8199 100644 > --- a/include/linux/platform_data/dma-dw.h > +++ b/include/linux/platform_data/dma-dw.h > @@ -37,10 +37,7 @@ struct dw_dma_slave { >  /** >   * struct dw_dma_platform_data - Controller configuration parameters >   * @nr_channels: Number of channels supported by hardware (max 8) > - * @is_private: The device channels should be marked as private and > not for > - * by the general purpose DMA channel allocator. > - * @is_memcpy: The device channels do support memory-to-memory > transfers. > - * @is_nollp: The device channels does not support multi block > transfers. > + * @quirks: Bit field with platform quirks >   * @chan_allocation_order: Allocate channels starting from 0 or 7 >   * @chan_priority: Set channel priority increasing from 0 to 7 or 7 > to 0. >   * @block_size: Maximum block size supported by the controller > @@ -50,9 +47,18 @@ struct dw_dma_slave { >   */ >  struct dw_dma_platform_data { >   unsigned int nr_channels; > - bool is_private; > - bool is_memcpy; > - bool is_nollp; > +/* Only use quirks from platform data structure */ > +#define QUIRKS_ONLY_USED 0 > +/* > + * The device channels should be marked as private and not for > + * by the general purpose DMA channel allocator. > + */ > +#define QUIRKS_IS_PRIVATE 1 > +/* The device channels do support memory-to-memory transfers. */ > +#define QUIRKS_IS_MEMCPY 2 > +/* The device channels do not support multi block transfers. */ > +#define QUIRKS_IS_NOLLP 3 > + unsigned long quirks; >  #define CHAN_ALLOCATION_ASCENDING 0 /* zero to seven > */ >  #define CHAN_ALLOCATION_DESCENDING 1 /* seven to zero > */ >   unsigned char chan_allocation_order; --  Paltsev Eugeniy