diff mbox

[RFC,01/11] dma: amba-pl08x: Use bitmap to pass variant specific quirks

Message ID 1371416058-22047-2-git-send-email-tomasz.figa@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tomasz Figa June 16, 2013, 8:54 p.m. UTC 
Instead of defining new bool field in vendor_data struct for each quirk,
it is more reasonable to use a single flags field and make each quirk
use single bits.

Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>
---
 drivers/dma/amba-pl08x.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

Comments

Russell King - ARM Linux June 17, 2013, 6:48 p.m. UTC | #1 
On Sun, Jun 16, 2013 at 10:54:08PM +0200, Tomasz Figa wrote:
> Instead of defining new bool field in vendor_data struct for each quirk,
> it is more reasonable to use a single flags field and make each quirk
> use single bits.

Please explain why this is better over the existing system, and why it is
not just churn for code modification's sake.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Figa June 17, 2013, 6:56 p.m. UTC | #2 
On Monday 17 of June 2013 19:48:56 Russell King - ARM Linux wrote:
> On Sun, Jun 16, 2013 at 10:54:08PM +0200, Tomasz Figa wrote:
> > Instead of defining new bool field in vendor_data struct for each
> > quirk, it is more reasonable to use a single flags field and make
> > each quirk use single bits.
> 
> Please explain why this is better over the existing system, and why it
> is not just churn for code modification's sake.

It isn't anything important. I just thought that this is a better solution 
to store more than just two flags. Initially I had more of them added in 
further patches, but final version ended with just one, so this can be 
dropped.

Best regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 8bad254..d443a68 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -93,18 +93,22 @@ 
 static struct amba_driver pl08x_amba_driver;
 struct pl08x_driver_data;
 
+/** Controller supports dual AHB masters. */
+#define PL08X_IS_DUALMASTER	(1 << 0)
+/**
+ * Controller has Nomadik security extension bits that need to be checked
+ * for permission before use and some registers are missing.
+ */
+#define PL08X_IS_NOMADIK	(1 << 1)
+
 /**
  * struct vendor_data - vendor-specific config parameters for PL08x derivatives
  * @channels: the number of channels available in this variant
- * @dualmaster: whether this version supports dual AHB masters or not.
- * @nomadik: whether the channels have Nomadik security extension bits
- *	that need to be checked for permission before use and some registers are
- *	missing
+ * @flags: Vendor-specific flags, see PL08X_IS_*
  */
 struct vendor_data {
 	u8 channels;
-	bool dualmaster;
-	bool nomadik;
+	u32 flags;
 };
 
 /*
@@ -1391,7 +1395,7 @@  static struct dma_async_tx_descriptor *pl08x_prep_dma_memcpy(
 	/* Both to be incremented or the code will break */
 	txd->cctl |= PL080_CONTROL_SRC_INCR | PL080_CONTROL_DST_INCR;
 
-	if (pl08x->vd->dualmaster)
+	if (pl08x->vd->flags & PL08X_IS_DUALMASTER)
 		txd->cctl |= pl08x_select_bus(pl08x->mem_buses,
 					      pl08x->mem_buses);
 
@@ -1612,7 +1616,7 @@  bool pl08x_filter_id(struct dma_chan *chan, void *chan_id)
 static void pl08x_ensure_on(struct pl08x_driver_data *pl08x)
 {
 	/* The Nomadik variant does not have the config register */
-	if (pl08x->vd->nomadik)
+	if (pl08x->vd->flags & PL08X_IS_NOMADIK)
 		return;
 	writel(PL080_CONFIG_ENABLE, pl08x->base + PL080_CONFIG);
 }
@@ -1897,7 +1901,7 @@  static int pl08x_probe(struct amba_device *adev, const struct amba_id *id)
 	/* By default, AHB1 only.  If dualmaster, from platform */
 	pl08x->lli_buses = PL08X_AHB1;
 	pl08x->mem_buses = PL08X_AHB1;
-	if (pl08x->vd->dualmaster) {
+	if (pl08x->vd->flags & PL08X_IS_DUALMASTER) {
 		pl08x->lli_buses = pl08x->pd->lli_buses;
 		pl08x->mem_buses = pl08x->pd->mem_buses;
 	}
@@ -1954,7 +1958,7 @@  static int pl08x_probe(struct amba_device *adev, const struct amba_id *id)
 		 * down for the secure world only. Lock up these channels
 		 * by perpetually serving a dummy virtual channel.
 		 */
-		if (vd->nomadik) {
+		if (vd->flags & PL08X_IS_NOMADIK) {
 			u32 val;
 
 			val = readl(ch->base + PL080_CH_CONFIG);
@@ -2039,18 +2043,17 @@  out_no_pl08x:
 /* PL080 has 8 channels and the PL080 have just 2 */
 static struct vendor_data vendor_pl080 = {
 	.channels = 8,
-	.dualmaster = true,
+	.flags = PL08X_IS_DUALMASTER,
 };
 
 static struct vendor_data vendor_nomadik = {
 	.channels = 8,
-	.dualmaster = true,
-	.nomadik = true,
+	.flags = PL08X_IS_DUALMASTER | PL08X_IS_NOMADIK,
 };
 
 static struct vendor_data vendor_pl081 = {
 	.channels = 2,
-	.dualmaster = false,
+	.flags = 0,
 };
 
 static struct amba_id pl08x_ids[] = {